diff --git a/src/services/Elastic.Changelog/GitHub/GitHubApiTransport.cs b/src/services/Elastic.Changelog/GitHub/GitHubApiTransport.cs new file mode 100644 index 000000000..8c9de024a --- /dev/null +++ b/src/services/Elastic.Changelog/GitHub/GitHubApiTransport.cs @@ -0,0 +1,104 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Net.Http.Headers; +using System.Text; + +namespace Elastic.Changelog.GitHub; + +/// +/// Shared HTTP transport for the GitHub API services (, +/// , ): one process-wide +/// , consistent User-Agent/Accept headers, GITHUB_TOKEN +/// bearer authentication, and an injectable so every consumer can +/// be tested at the HTTP level. Response handling policy (lenient warn-and-null vs strict +/// fail-the-run) stays with each service; this type only issues requests. +/// +public sealed class GitHubApiTransport : IDisposable +{ + private const string GraphQlEndpoint = "https://api.github.com/graphql"; + + private static readonly TimeSpan FetchTimeout = TimeSpan.FromSeconds(60); + + /// + /// Process-wide client shared by every transport built for the production (no injected handler) + /// path. Intentionally never disposed — it lives for the lifetime of the process. + /// + private static readonly HttpClient SharedHttpClient = CreateClient(null); + + private readonly HttpClient _httpClient; + + /// + /// Non-null only when a caller injects its own (tests): in that + /// case we own a per-instance client and must dispose it. On the production path + /// points at , which is never disposed. + /// + private readonly HttpClient? _ownedHttpClient; + private readonly string? _githubToken; + + /// Optional HTTP handler override (tests). Owned by the caller. + /// Optional token override; defaults to the GITHUB_TOKEN environment variable. + public GitHubApiTransport(HttpMessageHandler? handler = null, string? githubToken = null) + { + _githubToken = githubToken; + if (handler is null) + _httpClient = SharedHttpClient; + else + { + // disposeHandler: false — the injected handler is owned by the caller (tests), not by us. + _ownedHttpClient = CreateClient(handler); + _httpClient = _ownedHttpClient; + } + } + + private static HttpClient CreateClient(HttpMessageHandler? handler) + { + var client = handler is null + ? new HttpClient(new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5) }) + : new HttpClient(handler, disposeHandler: false); + client.Timeout = FetchTimeout; + client.DefaultRequestHeaders.Add("User-Agent", "docs-builder"); + return client; + } + + /// The effective token: the constructor override, else the GITHUB_TOKEN environment variable. + public string? ResolveToken() => _githubToken ?? Environment.GetEnvironmentVariable("GITHUB_TOKEN"); + + /// + /// Issues an authenticated (when a token resolves) GET against the GitHub REST API. + /// The caller owns the response and its status-code policy. + /// + public async Task GetAsync(string url, Cancel ctx = default) + { + using var request = new HttpRequestMessage(HttpMethod.Get, url); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.github+json")); + AttachAuthorization(request); + return await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); + } + + /// + /// Posts a JSON body to the GitHub GraphQL endpoint. The GraphQL API rejects anonymous + /// requests, so callers should verify before building queries. + /// + public async Task PostGraphQlAsync(string jsonBody, Cancel ctx = default) + { + using var request = new HttpRequestMessage(HttpMethod.Post, GraphQlEndpoint); + request.Content = new StringContent(jsonBody, Encoding.UTF8, "application/json"); + AttachAuthorization(request); + return await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); + } + + private void AttachAuthorization(HttpRequestMessage request) + { + var token = ResolveToken(); + if (!string.IsNullOrEmpty(token)) + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + } + + /// + /// Disposes the per-instance client created for an injected handler; the shared production + /// client is process-lived and intentionally not disposed. + /// + public void Dispose() => _ownedHttpClient?.Dispose(); +} diff --git a/src/services/Elastic.Changelog/GitHub/GitHubCommitRangeService.cs b/src/services/Elastic.Changelog/GitHub/GitHubCommitRangeService.cs index d63aa9fb1..be6ecddee 100644 --- a/src/services/Elastic.Changelog/GitHub/GitHubCommitRangeService.cs +++ b/src/services/Elastic.Changelog/GitHub/GitHubCommitRangeService.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information using System.Net; -using System.Net.Http.Headers; using System.Text; using System.Text.Json; using System.Text.Json.Serialization; @@ -19,24 +18,15 @@ namespace Elastic.Changelog.GitHub; /// (commit → PR association). Works for squash and merge commits on protected integration branches; /// commits with no associated merged PR are reported, not silently dropped. /// -public sealed partial class GitHubCommitRangeService : IGitHubCommitRangeService, IDisposable +public sealed partial class GitHubCommitRangeService(ILoggerFactory logFactory, GitHubApiTransport? transport = null) + : IGitHubCommitRangeService { private const int ComparePageSize = 100; private const int GraphQlBatchSize = 50; private const int MaxAssociatedPullRequests = 10; - private static readonly TimeSpan FetchTimeout = TimeSpan.FromSeconds(60); - - /// - /// Process-wide client shared by every service built for the production (no injected handler) - /// path. Intentionally never disposed — it lives for the lifetime of the process. - /// - private static readonly HttpClient SharedHttpClient = CreateClient(null); - - private readonly ILogger _logger; - private readonly HttpClient _httpClient; - private readonly HttpClient? _ownedHttpClient; - private readonly string? _githubToken; + private readonly ILogger _logger = logFactory.CreateLogger(); + private readonly GitHubApiTransport _transport = transport ?? new GitHubApiTransport(); [GeneratedRegex("^[0-9a-fA-F]{7,40}$")] private static partial Regex CommitShaRegex(); @@ -44,40 +34,13 @@ public sealed partial class GitHubCommitRangeService : IGitHubCommitRangeService [GeneratedRegex("^[A-Za-z0-9_.-]+$")] private static partial Regex SafeGraphQlIdentifierRegex(); - /// Logger factory. - /// Optional HTTP handler override (tests). Owned by the caller. - /// Optional token override; defaults to the GITHUB_TOKEN environment variable. - public GitHubCommitRangeService(ILoggerFactory logFactory, HttpMessageHandler? handler = null, string? githubToken = null) - { - _logger = logFactory.CreateLogger(); - _githubToken = githubToken; - if (handler is null) - _httpClient = SharedHttpClient; - else - { - // disposeHandler: false — the injected handler is owned by the caller (tests), not by us. - _ownedHttpClient = CreateClient(handler); - _httpClient = _ownedHttpClient; - } - } - - private static HttpClient CreateClient(HttpMessageHandler? handler) - { - var client = handler is null - ? new HttpClient(new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5) }) - : new HttpClient(handler, disposeHandler: false); - client.Timeout = FetchTimeout; - client.DefaultRequestHeaders.Add("User-Agent", "docs-builder"); - return client; - } - /// public async Task ResolvePullRequestsAsync( IDiagnosticsCollector collector, CommitRangeArguments args, Cancel ctx) { - var token = _githubToken ?? Environment.GetEnvironmentVariable("GITHUB_TOKEN"); + var token = _transport.ResolveToken(); if (string.IsNullOrWhiteSpace(token)) { collector.EmitError(string.Empty, @@ -93,7 +56,7 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) return null; } - var commits = await FetchCompareCommitsAsync(collector, args, token, ctx).ConfigureAwait(false); + var commits = await FetchCompareCommitsAsync(collector, args, ctx).ConfigureAwait(false); if (commits == null) return null; @@ -104,7 +67,7 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) return new CommitRangeResolution { TotalCommits = 0, PullRequests = [], CommitsWithoutPullRequest = [] }; } - return await AssociatePullRequestsAsync(collector, args, commits, token, ctx).ConfigureAwait(false); + return await AssociatePullRequestsAsync(collector, args, commits, ctx).ConfigureAwait(false); } /// @@ -114,7 +77,6 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) private async Task?> FetchCompareCommitsAsync( IDiagnosticsCollector collector, CommitRangeArguments args, - string token, Cancel ctx) { var basehead = $"{Uri.EscapeDataString(args.StartRef)}...{Uri.EscapeDataString(args.EndRef)}"; @@ -127,8 +89,7 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) var url = $"https://api.github.com/repos/{args.Owner}/{args.Repo}/compare/{basehead}?per_page={ComparePageSize}&page={page}"; _logger.LogDebug("Fetching compare page {Page}: {Url}", page, url); - using var request = CreateRestRequest(url, token); - using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); + using var response = await _transport.GetAsync(url, ctx).ConfigureAwait(false); if (response.StatusCode == HttpStatusCode.NotFound) { collector.EmitError(string.Empty, @@ -203,7 +164,6 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) IDiagnosticsCollector collector, CommitRangeArguments args, IReadOnlyList commits, - string token, Cancel ctx) { var invalidShas = commits.Where(sha => !CommitShaRegex().IsMatch(sha)).ToList(); @@ -222,7 +182,7 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) for (var offset = 0; offset < commits.Count; offset += GraphQlBatchSize) { var batch = commits.Skip(offset).Take(GraphQlBatchSize).ToList(); - var byAlias = await FetchAssociatedPullRequestsBatchAsync(collector, args, batch, token, ctx).ConfigureAwait(false); + var byAlias = await FetchAssociatedPullRequestsBatchAsync(collector, args, batch, ctx).ConfigureAwait(false); if (byAlias == null) return null; @@ -314,17 +274,12 @@ private static HttpClient CreateClient(HttpMessageHandler? handler) IDiagnosticsCollector collector, CommitRangeArguments args, IReadOnlyList shas, - string token, Cancel ctx) { var query = BuildBatchQuery(args.Owner, args.Repo, shas); var body = JsonSerializer.Serialize(new GraphQlRequest { Query = query }, CommitRangeJsonContext.Default.GraphQlRequest); - using var request = new HttpRequestMessage(HttpMethod.Post, "https://api.github.com/graphql"); - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); - request.Content = new StringContent(body, Encoding.UTF8, "application/json"); - - using var response = await _httpClient.SendAsync(request, ctx).ConfigureAwait(false); + using var response = await _transport.PostGraphQlAsync(body, ctx).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { collector.EmitError(string.Empty, @@ -372,20 +327,6 @@ private static string BuildBatchQuery(string owner, string repo, IReadOnlyList - /// Disposes the per-instance client created for an injected handler; the shared production - /// client is process-lived and intentionally not disposed. - /// - public void Dispose() => _ownedHttpClient?.Dispose(); - private sealed class GitHubCompareResponse { [JsonPropertyName("status")] diff --git a/src/services/Elastic.Changelog/GitHub/GitHubPrService.cs b/src/services/Elastic.Changelog/GitHub/GitHubPrService.cs index 9a537742f..c65a8d543 100644 --- a/src/services/Elastic.Changelog/GitHub/GitHubPrService.cs +++ b/src/services/Elastic.Changelog/GitHub/GitHubPrService.cs @@ -13,16 +13,10 @@ namespace Elastic.Changelog.GitHub; /// /// Service for fetching pull request information from GitHub /// -public partial class GitHubPrService(ILoggerFactory loggerFactory) : IGitHubPrService +public partial class GitHubPrService(ILoggerFactory loggerFactory, GitHubApiTransport? transport = null) : IGitHubPrService { private readonly ILogger _logger = loggerFactory.CreateLogger(); - private static readonly HttpClient HttpClient = new(); - - static GitHubPrService() - { - HttpClient.DefaultRequestHeaders.Add("User-Agent", "docs-builder"); - HttpClient.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json"); - } + private readonly GitHubApiTransport _transport = transport ?? new GitHubApiTransport(); /// /// Fetches pull request information from GitHub @@ -43,15 +37,10 @@ static GitHubPrService() return null; } - // Add GitHub token if available (for rate limiting and private repos) - var githubToken = Environment.GetEnvironmentVariable("GITHUB_TOKEN"); - using var request = new HttpRequestMessage(HttpMethod.Get, $"https://api.github.com/repos/{parsedOwner}/{parsedRepo}/pulls/{prNumber}"); - if (!string.IsNullOrEmpty(githubToken)) - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", githubToken); - - _logger.LogDebug("Fetching PR info from: {ApiUrl}", request.RequestUri); + var url = $"https://api.github.com/repos/{parsedOwner}/{parsedRepo}/pulls/{prNumber}"; + _logger.LogDebug("Fetching PR info from: {ApiUrl}", url); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogWarning("Failed to fetch PR info. Status: {StatusCode}, Reason: {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); @@ -195,14 +184,10 @@ private static IReadOnlyList ExtractLinkedIssues(string body, string prO return null; } - var githubToken = Environment.GetEnvironmentVariable("GITHUB_TOKEN"); - using var request = new HttpRequestMessage(HttpMethod.Get, $"https://api.github.com/repos/{parsedOwner}/{parsedRepo}/issues/{issueNumber}"); - if (!string.IsNullOrEmpty(githubToken)) - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", githubToken); + var url = $"https://api.github.com/repos/{parsedOwner}/{parsedRepo}/issues/{issueNumber}"; + _logger.LogDebug("Fetching issue info from: {ApiUrl}", url); - _logger.LogDebug("Fetching issue info from: {ApiUrl}", request.RequestUri); - - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogWarning("Failed to fetch issue info. Status: {StatusCode}, Reason: {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); @@ -250,10 +235,10 @@ private static IReadOnlyList ExtractLinkedIssues(string body, string prO { try { - using var request = CreateRequest(HttpMethod.Get, $"https://api.github.com/repos/{owner}/{repo}/commits/{sha}"); - _logger.LogDebug("Fetching commit author from: {ApiUrl}", request.RequestUri); + var url = $"https://api.github.com/repos/{owner}/{repo}/commits/{sha}"; + _logger.LogDebug("Fetching commit author from: {ApiUrl}", url); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogWarning("Failed to fetch commit info. Status: {StatusCode}", response.StatusCode); @@ -277,10 +262,9 @@ private static IReadOnlyList ExtractLinkedIssues(string body, string prO try { var url = $"https://api.github.com/repos/{owner}/{repo}/commits?path={Uri.EscapeDataString(filePath)}&sha={Uri.EscapeDataString(branch)}&per_page=1"; - using var request = CreateRequest(HttpMethod.Get, url); - _logger.LogDebug("Fetching last file commit author from: {ApiUrl}", request.RequestUri); + _logger.LogDebug("Fetching last file commit author from: {ApiUrl}", url); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogWarning("Failed to fetch file commit history. Status: {StatusCode}", response.StatusCode); @@ -301,15 +285,6 @@ private static IReadOnlyList ExtractLinkedIssues(string body, string prO } } - private static HttpRequestMessage CreateRequest(HttpMethod method, string url) - { - var request = new HttpRequestMessage(method, url); - var githubToken = Environment.GetEnvironmentVariable("GITHUB_TOKEN"); - if (!string.IsNullOrEmpty(githubToken)) - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", githubToken); - return request; - } - private static (string? owner, string? repo, int? issueNumber) ParseIssueUrl(string issueUrl, string? defaultOwner = null, string? defaultRepo = null) { if (issueUrl.StartsWith("https://github.com/", StringComparison.OrdinalIgnoreCase) || diff --git a/src/services/Elastic.Changelog/GitHub/GitHubReleaseService.cs b/src/services/Elastic.Changelog/GitHub/GitHubReleaseService.cs index bd92317e4..d3132576f 100644 --- a/src/services/Elastic.Changelog/GitHub/GitHubReleaseService.cs +++ b/src/services/Elastic.Changelog/GitHub/GitHubReleaseService.cs @@ -12,16 +12,10 @@ namespace Elastic.Changelog.GitHub; /// /// Service for fetching release information from GitHub /// -public partial class GitHubReleaseService(ILoggerFactory loggerFactory) : IGitHubReleaseService +public partial class GitHubReleaseService(ILoggerFactory loggerFactory, GitHubApiTransport? transport = null) : IGitHubReleaseService { private readonly ILogger _logger = loggerFactory.CreateLogger(); - private static readonly HttpClient HttpClient = new(); - - static GitHubReleaseService() - { - HttpClient.DefaultRequestHeaders.Add("User-Agent", "docs-builder"); - HttpClient.DefaultRequestHeaders.Add("Accept", "application/vnd.github.v3+json"); - } + private readonly GitHubApiTransport _transport = transport ?? new GitHubApiTransport(); /// public async Task FetchReleaseAsync( @@ -79,10 +73,9 @@ public async Task> FetchReleasesAsync( try { var url = $"https://api.github.com/repos/{owner}/{repo}/releases?per_page={count}"; - using var request = CreateRequest(url); _logger.LogDebug("Fetching releases from: {ApiUrl}", url); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogDebug("Failed to fetch releases. Status: {StatusCode}, Reason: {ReasonPhrase}", @@ -111,10 +104,9 @@ public async Task> FetchReleasesAsync( { try { - using var request = CreateRequest(asset.BrowserDownloadUrl); _logger.LogDebug("Downloading release asset: {AssetUrl}", asset.BrowserDownloadUrl); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(asset.BrowserDownloadUrl, ctx); if (!response.IsSuccessStatusCode) { _logger.LogDebug("Failed to download asset {AssetName}. Status: {StatusCode}, Reason: {ReasonPhrase}", @@ -136,22 +128,11 @@ public async Task> FetchReleasesAsync( } } - private static HttpRequestMessage CreateRequest(string url) - { - // Add GitHub token if available (for rate limiting and private repos) - var githubToken = Environment.GetEnvironmentVariable("GITHUB_TOKEN"); - var request = new HttpRequestMessage(HttpMethod.Get, url); - if (!string.IsNullOrEmpty(githubToken)) - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", githubToken); - return request; - } - private async Task FetchReleaseFromUrl(string url, CancellationToken ctx) { - using var request = CreateRequest(url); _logger.LogDebug("Fetching release info from: {ApiUrl}", url); - var response = await HttpClient.SendAsync(request, ctx); + using var response = await _transport.GetAsync(url, ctx); if (!response.IsSuccessStatusCode) { _logger.LogDebug("Failed to fetch release info. Status: {StatusCode}, Reason: {ReasonPhrase}", diff --git a/tests/Elastic.Changelog.Tests/Changelogs/GitHubCommitRangeServiceTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/GitHubCommitRangeServiceTests.cs index 6a6d5c8a8..07e8b55ca 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/GitHubCommitRangeServiceTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/GitHubCommitRangeServiceTests.cs @@ -60,7 +60,7 @@ private static string GraphQlJson(IReadOnlyList<(string Sha, string[] PrNodes)> } private GitHubCommitRangeService Service(StubHandler handler) => - new(new TestLoggerFactory(Output), handler, githubToken: "test-token"); + new(new TestLoggerFactory(Output), new GitHubApiTransport(handler, "test-token")); private static StubHandler Handler(Func compareResponder, Func graphQlResponder) => new(req => @@ -244,7 +244,7 @@ public async Task ResolvePullRequests_UnknownRefs_EmitsErrorAndReturnsNull() public async Task ResolvePullRequests_MissingToken_EmitsErrorWithoutAnyRequest() { var handler = Handler(_ => throw new InvalidOperationException("no request expected"), _ => throw new InvalidOperationException("no request expected")); - var service = new GitHubCommitRangeService(new TestLoggerFactory(Output), handler, githubToken: ""); + var service = new GitHubCommitRangeService(new TestLoggerFactory(Output), new GitHubApiTransport(handler, "")); var result = await service.ResolvePullRequestsAsync(Collector, Args, TestContext.Current.CancellationToken);