Clarify entity fetcher contract #192
Conversation
lorensr
commented
Nov 13, 2024
lorensr
commented
- Should return either a single instance or null (technically can return a List with one item in it and the framework will unwrap, but not mentioning that as I wouldn't recommend it)
- Can fetch data
|
|
||
| In this case, we're not doing any data fetching: our `Show` instance only has an `id` field, and we implement a `Show.reviews` datafetcher in the [next section](#providing-data-with-a-data-fetcher). | ||
|
|
||
| However, we could instead fetch data in the entity fetcher. If our DGS served multiple fields, and they all came from the same data source, we could fetch them all at once in the entity fetcher instead of writing separate datafetchers for each field: |
There was a problem hiding this comment.
I would leave this out from the docs - don't think we want to necessarily recommend this as a pattern.
There was a problem hiding this comment.
So is it an anti-pattern to build the whole entity object? Should we always return the partial entity (only id field)?
If it is, maybe it makes sense to mentioned that it is not recommended.
There was a problem hiding this comment.
IMO it's a valid thing to do, and sometimes the better thing to do, and many won't realize it's an option unless we say it is. Maybe can change/add language around it to say only use it in this circumstance, or "here is why you often don't want to do it?"
There was a problem hiding this comment.
Ok, I guess if it's providing multiple federated fields - so would be good to highlight that.
There was a problem hiding this comment.
+1 that this is a valid pattern
| The DGS framework does most of the heavy lifting, and all we have to do is provide the following: | ||
| The Reviews DGS needs to implement an entity fetcher to handle this query. | ||
|
|
||
| - **Entity fetcher**: A method annotated with [`@DgsEntityFetcher`](https://javadoc.io/doc/com.netflix.graphql.dgs/graphql-dgs/latest/com/netflix/graphql/dgs/DgsEntityFetcher.html) that takes a key and returns a single instance of the entity or null. |
There was a problem hiding this comment.
what are the implications of returning the entity or null? which one is preferred?
There was a problem hiding this comment.
I think it also makes sense to mention that the entity resolver must return some value for every key.
|
|
||
| ```java | ||
| @DgsEntityFetcher(name = "Show") | ||
| public Show movie(Map<String, Object> values, DataFetchingEnvironment env) { |
There was a problem hiding this comment.
it should be CompletableFuture<Show>
There was a problem hiding this comment.
Why? I don't think it needs to be?
There was a problem hiding this comment.
doesn't dataLoader.load(showId) return a Future?
| } | ||
| ``` | ||
|
|
||
| If there is no such Show with the given id in the database, the entity fetcher should return `null`. |
There was a problem hiding this comment.
Maybe explain a bit further what this means for the federated request. E.g. will this show as an error
|
|
||
| In this case, we're not doing any data fetching: our `Show` instance only has an `id` field, and we implement a `Show.reviews` datafetcher in the [next section](#providing-data-with-a-data-fetcher). | ||
|
|
||
| However, we could instead fetch data in the entity fetcher. If our DGS served multiple fields, and they all came from the same data source, we could fetch them all at once in the entity fetcher instead of writing separate datafetchers for each field: |
There was a problem hiding this comment.
So is it an anti-pattern to build the whole entity object? Should we always return the partial entity (only id field)?
If it is, maybe it makes sense to mentioned that it is not recommended.
| @@ -173,7 +198,7 @@ public List<Review> reviews(DgsDataFetchingEnvironment dataFetchingEnvironment) | |||
|
|
|||
| ### Testing a Federated Query | |||
|
|
|||
| You can always manually test federated queries by running the gateway and your DGS locally. | |||
| You can always manually test federated queries by running the gateway and your DGS locally. | |||
There was a problem hiding this comment.
I think we should flip this text around and focus on testing without running a gateway, so:
- Write a test
- Test by sending a federated query directly to the DGS (e.g. using graphiql)
- Run your gateway locally
