-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Export GraphQLFeaturesTransport and WithGraphQLFeatures for remote server support #1809
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
Closed
Closed
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0ea475c
Initial plan
Copilot 249755f
Add GraphQLFeaturesTransport for reusable header handling
Copilot fc95d3e
Improve documentation for GraphQL-Features transport
Copilot 7333546
Move GraphQL features functions to transport.go
Copilot 27ee7e4
Export WithGraphQLFeatures for remote server tests
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package github | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "strings" | ||
| ) | ||
|
|
||
| // graphQLFeaturesKey is a context key for GraphQL feature flags. | ||
| // These flags enable preview or experimental GitHub API features that are not yet GA. | ||
| type graphQLFeaturesKey struct{} | ||
|
|
||
| // withGraphQLFeatures adds GraphQL feature flags to the context. | ||
| // The flags are read by GraphQLFeaturesTransport and sent as the GraphQL-Features header. | ||
| // This is used internally by tool handlers that require experimental GitHub API features. | ||
| func withGraphQLFeatures(ctx context.Context, features ...string) context.Context { | ||
| return context.WithValue(ctx, graphQLFeaturesKey{}, features) | ||
| } | ||
|
|
||
| // GetGraphQLFeatures retrieves GraphQL feature flags from the context. | ||
| // This function is exported to allow custom HTTP transports (e.g., in remote servers) | ||
| // to read feature flags and add them as the "GraphQL-Features" header. | ||
| // | ||
| // For most use cases, use GraphQLFeaturesTransport instead of calling this directly. | ||
| func GetGraphQLFeatures(ctx context.Context) []string { | ||
| if features, ok := ctx.Value(graphQLFeaturesKey{}).([]string); ok { | ||
| return features | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // GraphQLFeaturesTransport is an http.RoundTripper that adds GraphQL-Features | ||
| // header based on context values set by withGraphQLFeatures. | ||
| // | ||
| // This transport should be used in the HTTP client chain for githubv4.Client | ||
| // to ensure GraphQL feature flags are properly sent to the GitHub API. | ||
| // Without this transport, certain GitHub API features (like Copilot assignment) | ||
| // that require feature flags will fail with schema validation errors. | ||
| // | ||
| // Example usage for local server (layering with auth): | ||
| // | ||
| // httpClient := &http.Client{ | ||
| // Transport: &github.GraphQLFeaturesTransport{ | ||
| // Transport: &authTransport{ | ||
| // Transport: http.DefaultTransport, | ||
| // token: "ghp_...", | ||
| // }, | ||
| // }, | ||
| // } | ||
| // gqlClient := githubv4.NewClient(httpClient) | ||
| // | ||
| // Example usage for remote server (simple case): | ||
| // | ||
| // httpClient := &http.Client{ | ||
| // Transport: &github.GraphQLFeaturesTransport{ | ||
| // Transport: http.DefaultTransport, | ||
| // }, | ||
| // } | ||
| // gqlClient := githubv4.NewClient(httpClient) | ||
| // | ||
| // The transport reads feature flags from request context using GetGraphQLFeatures. | ||
| // Feature flags are added to context by the tool handler via withGraphQLFeatures. | ||
| type GraphQLFeaturesTransport struct { | ||
| // Transport is the underlying http.RoundTripper. If nil, http.DefaultTransport is used. | ||
| Transport http.RoundTripper | ||
| } | ||
|
|
||
| // RoundTrip implements http.RoundTripper. | ||
| // It adds the GraphQL-Features header if features are present in the request context. | ||
| func (t *GraphQLFeaturesTransport) RoundTrip(req *http.Request) (*http.Response, error) { | ||
| transport := t.Transport | ||
| if transport == nil { | ||
| transport = http.DefaultTransport | ||
| } | ||
|
|
||
| // Clone request to avoid modifying the original | ||
| req = req.Clone(req.Context()) | ||
|
|
||
| // Check for GraphQL-Features in context and add header if present | ||
| if features := GetGraphQLFeatures(req.Context()); len(features) > 0 { | ||
| req.Header.Set("GraphQL-Features", strings.Join(features, ", ")) | ||
| } | ||
|
|
||
| return transport.RoundTrip(req) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| package github | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestGraphQLFeaturesTransport(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| features []string | ||
| expectHeader bool | ||
| expectedHeaderVal string | ||
| }{ | ||
| { | ||
| name: "adds single feature to header", | ||
| features: []string{"issues_copilot_assignment_api_support"}, | ||
| expectHeader: true, | ||
| expectedHeaderVal: "issues_copilot_assignment_api_support", | ||
| }, | ||
| { | ||
| name: "adds multiple features to header", | ||
| features: []string{"feature1", "feature2", "feature3"}, | ||
| expectHeader: true, | ||
| expectedHeaderVal: "feature1, feature2, feature3", | ||
| }, | ||
| { | ||
| name: "no header when no features in context", | ||
| features: nil, | ||
| expectHeader: false, | ||
| }, | ||
| { | ||
| name: "no header when empty features slice", | ||
| features: []string{}, | ||
| expectHeader: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Create a test server that captures the request | ||
| var capturedReq *http.Request | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| capturedReq = r | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| // Create HTTP client with GraphQLFeaturesTransport | ||
| client := &http.Client{ | ||
| Transport: &GraphQLFeaturesTransport{ | ||
| Transport: http.DefaultTransport, | ||
| }, | ||
| } | ||
|
|
||
| // Create request with or without features in context | ||
| ctx := context.Background() | ||
| if tt.features != nil { | ||
| ctx = withGraphQLFeatures(ctx, tt.features...) | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| // Make request | ||
| resp, err := client.Do(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
|
|
||
| // Verify header | ||
| if tt.expectHeader { | ||
| assert.Equal(t, tt.expectedHeaderVal, capturedReq.Header.Get("GraphQL-Features")) | ||
| } else { | ||
| assert.Empty(t, capturedReq.Header.Get("GraphQL-Features")) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestGraphQLFeaturesTransport_NilTransport(t *testing.T) { | ||
| // Test that nil Transport falls back to http.DefaultTransport | ||
| var capturedReq *http.Request | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| capturedReq = r | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| client := &http.Client{ | ||
| Transport: &GraphQLFeaturesTransport{ | ||
| Transport: nil, // Explicitly nil | ||
| }, | ||
| } | ||
|
|
||
| ctx := withGraphQLFeatures(context.Background(), "test_feature") | ||
| req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| resp, err := client.Do(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
|
|
||
| assert.Equal(t, "test_feature", capturedReq.Header.Get("GraphQL-Features")) | ||
| } | ||
|
|
||
| func TestGraphQLFeaturesTransport_PreservesOtherHeaders(t *testing.T) { | ||
| // Test that the transport doesn't interfere with other headers | ||
| var capturedReq *http.Request | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| capturedReq = r | ||
| w.WriteHeader(http.StatusOK) | ||
| })) | ||
| defer server.Close() | ||
|
|
||
| client := &http.Client{ | ||
| Transport: &GraphQLFeaturesTransport{ | ||
| Transport: http.DefaultTransport, | ||
| }, | ||
| } | ||
|
|
||
| ctx := withGraphQLFeatures(context.Background(), "feature1") | ||
| req, err := http.NewRequestWithContext(ctx, "GET", server.URL, nil) | ||
| require.NoError(t, err) | ||
|
|
||
| // Add custom headers | ||
| req.Header.Set("Authorization", "Bearer test-token") | ||
| req.Header.Set("User-Agent", "test-agent") | ||
|
|
||
| resp, err := client.Do(req) | ||
| require.NoError(t, err) | ||
| defer resp.Body.Close() | ||
|
|
||
| // Verify all headers are preserved | ||
| assert.Equal(t, "feature1", capturedReq.Header.Get("GraphQL-Features")) | ||
| assert.Equal(t, "Bearer test-token", capturedReq.Header.Get("Authorization")) | ||
| assert.Equal(t, "test-agent", capturedReq.Header.Get("User-Agent")) | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot export this please, so the remote server can use in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exported! Renamed
withGraphQLFeaturestoWithGraphQLFeaturesand updated all references. Remote server can now use it in tests to simulate feature flag contexts. Commit: 27ee7e4