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

Support for multiple @key directives in federation #1684

Merged

Conversation

carldunham
Copy link
Contributor

Describe your PR and link to any relevant issues.

Adds support for multiple @key directives for OBJECT types, which is allowed for in the Apollo spec. Can be useful in cases where some subgraphs have different pieces of data for an external type that can be resolved in multiple ways.

Should also open the door for #1670 to be implemented, and remove another * from apollographql/apollo-federation-subgraph-compatibility#42.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

coveralls commented Nov 2, 2021

Coverage Status

Coverage increased (+0.5%) to 54.577% when pulling 85beda0 on carldunham:cd/mutiple-key-directives into 59a3091 on 99designs:master.

@carldunham carldunham force-pushed the cd/mutiple-key-directives branch from 7ce0f28 to 85beda0 Compare November 3, 2021 01:41
@StevenACoffman StevenACoffman merged commit 47de912 into 99designs:master Nov 3, 2021
@StevenACoffman
Copy link
Collaborator

Thanks for this!

MiguelCastillo added a commit to Khan/gqlgen that referenced this pull request Nov 8, 2021
The tests work by sending `_entities` queries with `representation` variables directly to the mocked server, which will allow us to test the generated end to end.  For context, the format of the entity query is something like:

```
query($representations:[_Any!]!){_entities(representations:$representations){ ...on Hello{secondary} }}
```

And `representations` are the list of federated keys for the entities being resovled, and they look like

```
representations: [{
   "__typename": "Hello",
   "name":       "federated key value 1",
}, {
   "__typename": "Hello",
   "name":       "federated key value 2",
}]
```

The entity resolver tests are in `plugin/federation/federation_entityresolver_test.go` and they rely on `plugin/federation/testdata/entityresolver`.

NOTE: currently there are two regressions from 99designs#1684.
1. The generated code federation plugin creates a self executing function for resolving entities. The issue is that the self executing function specifies the graphql entity the resolver returns and it includes the namespace. So generating federation.go in the same package as the generated models causes compile errors.  See plugin/federation/testdata/entityresolver/federation.gp:70.
2. When there are multiple federated keys on an entity and one is nested, depending on the order in which the entities are resolved there can be exceptions.

Because of the first issue, the generated code has to be manually changed to remove the namespace `generated` from `testdata/entityresolver/generated/federation.go`. Then you should be able to run the tests.  The second test fails because of issue two.

To run the tests:
1. Build the entityresolver testdata
  - From plugin/federation, run `go run github.com/99designs/gqlgen --config testdata/entityresolver/gqlgen.yml`
2. Run the tests with `go test ./...` or similar
@MiguelCastillo MiguelCastillo mentioned this pull request Nov 8, 2021
2 tasks
@carldunham carldunham deleted the cd/mutiple-key-directives branch November 8, 2021 16:46
StevenACoffman added a commit that referenced this pull request Nov 8, 2021
StevenACoffman added a commit that referenced this pull request Nov 8, 2021
MiguelCastillo added a commit to Khan/gqlgen that referenced this pull request Nov 9, 2021
The tests work by sending `_entities` queries with `representation` variables directly to the mocked server, which will allow us to test the generated end to end.  For context, the format of the entity query is something like:

```
query($representations:[_Any!]!){_entities(representations:$representations){ ...on Hello{secondary} }}
```

And `representations` are the list of federated keys for the entities being resovled, and they look like

```
representations: [{
   "__typename": "Hello",
   "name":       "federated key value 1",
}, {
   "__typename": "Hello",
   "name":       "federated key value 2",
}]
```

The entity resolver tests are in `plugin/federation/federation_entityresolver_test.go` and they rely on `plugin/federation/testdata/entityresolver`.

NOTE: currently there are two regressions from 99designs#1684.
1. The generated code federation plugin creates a self executing function for resolving entities. The issue is that the self executing function specifies the graphql entity the resolver returns and it includes the namespace. So generating federation.go in the same package as the generated models causes compile errors.  See plugin/federation/testdata/entityresolver/federation.gp:70.
2. When there are multiple federated keys on an entity and one is nested, depending on the order in which the entities are resolved there can be exceptions.

Because of the first issue, the generated code has to be manually changed to remove the namespace `generated` from `testdata/entityresolver/generated/federation.go`. Then you should be able to run the tests.  The second test fails because of issue two.

To run the tests:
1. Build the entityresolver testdata
  - From plugin/federation, run `go run github.com/99designs/gqlgen --config testdata/entityresolver/gqlgen.yml`
2. Run the tests with `go test ./...` or similar
@StevenACoffman StevenACoffman added the federation Related to Apollo federation label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
federation Related to Apollo federation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants