Skip to content

Deduplicate OAuth2 provider HTTP client fallback#328

Merged
veverkap merged 2 commits into
mainfrom
copilot/fix-duplicate-httpclient-methods
May 23, 2026
Merged

Deduplicate OAuth2 provider HTTP client fallback#328
veverkap merged 2 commits into
mainfrom
copilot/fix-duplicate-httpclient-methods

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 23, 2026

Both built-in OAuth2 providers carried the same HTTPClient nil-fallback logic, creating unnecessary duplication in a shared request path. This change consolidates that behavior so future client customization stays in one place.

  • Shared HTTP client resolution

    • Introduces a package-level resolveHTTPClient helper in handler/oauth2_providers.go
    • Centralizes the nil -> http.DefaultClient fallback used by built-in OAuth2 providers
  • Provider cleanup

    • Removes the duplicated httpClient() methods from:
      • GitHubProvider
      • GoogleOAuth2Provider
    • Updates provider request code to call the shared helper directly
  • Focused coverage

    • Adds a small unit test covering:
      • returning the injected client when present
      • falling back to http.DefaultClient when absent
func resolveHTTPClient(client *http.Client) *http.Client {
	if client != nil {
		return client
	}
	return http.DefaultClient
}

Greptile Summary

This PR removes duplicated httpClient() methods from GitHubProvider and GoogleOAuth2Provider by extracting their shared nil-fallback logic into a single package-level resolveHTTPClient helper.

  • Introduces resolveHTTPClient(*http.Client) *http.Client in handler/oauth2_providers.go, replacing two identical receiver methods with a single shared implementation.
  • Adds a focused unit test (handler/oauth2_providers_test.go) covering both the injected-client and http.DefaultClient fallback paths.

Confidence Score: 5/5

Safe to merge — this is a pure refactor with no behavior change.

The extracted resolveHTTPClient helper is logically identical to the two methods it replaces, all three call sites are updated consistently, and the new tests confirm both branches. There is no changed behavior, no new dependency, and no untested path.

No files require special attention.

Important Files Changed

Filename Overview
handler/oauth2_providers.go Removes duplicate httpClient() receiver methods from both providers and replaces call sites with the new package-level resolveHTTPClient helper; behavior is identical to before.
handler/oauth2_providers_test.go New test file with two sub-tests for resolveHTTPClient; uses require.Same for pointer-equality checks, correctly covering both the injected-client and nil-fallback branches.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Provider method\n(fetchGitHubUser /\nfetchGitHubPrimaryEmail /\nFetchUserInfo)"] --> B["resolveHTTPClient(p.HTTPClient)"]
    B --> C{client != nil?}
    C -- Yes --> D["Return injected *http.Client"]
    C -- No --> E["Return http.DefaultClient"]
    D --> F["client.Do(req)"]
    E --> F
Loading

Reviews (1): Last reviewed commit: "refactor: share oauth2 provider HTTP cli..." | Re-trigger Greptile

Copilot AI changed the title [WIP] Fix duplicate httpClient helper method in OAuth2 providers Deduplicate OAuth2 provider HTTP client fallback May 23, 2026
Copilot AI requested a review from veverkap May 23, 2026 15:41
@veverkap veverkap marked this pull request as ready for review May 23, 2026 16:15
@veverkap veverkap requested review from a team and Copilot May 23, 2026 16:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR deduplicates the nil -> http.DefaultClient fallback logic for OAuth2 provider HTTP clients by introducing a shared helper in the provider implementation file, keeping built-in provider request paths consistent and easier to maintain.

Changes:

  • Added a package-level resolveHTTPClient(*http.Client) *http.Client helper to centralize defaulting behavior.
  • Removed duplicated per-provider httpClient() methods from GitHubProvider and GoogleOAuth2Provider, updating request execution to use the shared helper.
  • Added a unit test to cover both the “provided client” and “nil fallback” cases.
Show a summary per file
File Description
handler/oauth2_providers.go Centralizes HTTP client fallback logic and updates built-in providers to use it.
handler/oauth2_providers_test.go Adds targeted unit test coverage for the shared HTTP client resolution helper.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@veverkap veverkap merged commit 2618f9f into main May 23, 2026
33 checks passed
@veverkap veverkap deleted the copilot/fix-duplicate-httpclient-methods branch May 23, 2026 17:38
@github-actions github-actions Bot mentioned this pull request May 23, 2026
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

Successfully merging this pull request may close these issues.

Duplicate Code: httpClient() Helper Method in OAuth2 Providers

3 participants