From c08978340f7877d5008dddec823b7be00076f540 Mon Sep 17 00:00:00 2001 From: Zefek Date: Sat, 19 Sep 2026 14:57:34 +0200 Subject: [PATCH] Stop building the invoker twice in the sample The sample logged which git-http-backend was found by constructing a second GitHttpBackendInvoker, which ran GitBackendLocator.Locate() again -- another git --exec-path process and another round of file-existence checks -- two files away from a comment saying the invoker is constructed once. MapGitHttpBackend gains an overload taking an invoker the caller already built. The invoker exposes the options it was constructed with, so the overload needs nothing else and HandleAsync reads the hook off the instance it was handed. The existing (prefix, options) overload keeps its signature and behaviour and now delegates to the new one, so nothing breaks for current callers. The sample builds one invoker, logs its BackendPath and maps it, which also means a bad backend path now fails before the log line rather than after it. Refs #14 --- samples/GitHttpBackend.Server/Program.cs | 9 ++- .../GitHttpBackendEndpointExtensions.cs | 26 +++++- src/GitHttpBackend/GitHttpBackendInvoker.cs | 7 ++ .../CallerOwnedInvokerTests.cs | 81 +++++++++++++++++++ .../GitTestServer.cs | 14 +++- 5 files changed, 130 insertions(+), 7 deletions(-) create mode 100644 tests/GitHttpBackend.AspNetCore.Tests/CallerOwnedInvokerTests.cs diff --git a/samples/GitHttpBackend.Server/Program.cs b/samples/GitHttpBackend.Server/Program.cs index 6bac035..442c7d5 100644 --- a/samples/GitHttpBackend.Server/Program.cs +++ b/samples/GitHttpBackend.Server/Program.cs @@ -101,7 +101,12 @@ return Results.Content(RenderHomePage(projectRoot, ctx.Request, canAccess), "text/html; charset=utf-8"); }); -var endpoint = app.MapGitHttpBackend("/", options); +// Built here rather than inside MapGitHttpBackend so the resolved backend path is available +// for the startup log without resolving it a second time — and so a bad path fails before +// that line is written, not after it. +var invoker = new GitHttpBackendInvoker(options); + +var endpoint = app.MapGitHttpBackend("/", invoker); if (useBasic) { endpoint.RequireAuthorization(); @@ -110,7 +115,7 @@ app.Logger.LogInformation( "Serving git repositories from {ProjectRoot} (auth mode: {AuthMode}, git-http-backend: {BackendPath})", - projectRoot, useBasic ? "basic" : "none", new GitHttpBackendInvoker(options).BackendPath); + projectRoot, useBasic ? "basic" : "none", invoker.BackendPath); await app.RunAsync(); diff --git a/src/GitHttpBackend.AspNetCore/GitHttpBackendEndpointExtensions.cs b/src/GitHttpBackend.AspNetCore/GitHttpBackendEndpointExtensions.cs index 93df1b5..e847c3a 100644 --- a/src/GitHttpBackend.AspNetCore/GitHttpBackendEndpointExtensions.cs +++ b/src/GitHttpBackend.AspNetCore/GitHttpBackendEndpointExtensions.cs @@ -23,17 +23,37 @@ public static IEndpointConventionBuilder MapGitHttpBackend( ArgumentNullException.ThrowIfNull(options); // Constructed once: resolves and validates the backend path up front. - var invoker = new GitHttpBackendInvoker(options); + return endpoints.MapGitHttpBackend(prefix, new GitHttpBackendInvoker(options)); + } + + /// + /// Maps Git Smart HTTP endpoints under using an invoker the + /// caller already built. + /// + /// + /// Resolving git-http-backend starts a git --exec-path process and checks the + /// filesystem, so a host that also wants the resolved path — to log it at startup, say — + /// can build the invoker itself, read , and + /// hand the same instance here rather than paying for the lookup twice. It also means a + /// bad backend path fails before that log line rather than after it. + /// + public static IEndpointConventionBuilder MapGitHttpBackend( + this IEndpointRouteBuilder endpoints, string prefix, GitHttpBackendInvoker invoker) + { + ArgumentNullException.ThrowIfNull(endpoints); + ArgumentNullException.ThrowIfNull(invoker); var normalizedPrefix = "/" + prefix.Trim('/'); var pattern = (normalizedPrefix == "/" ? "" : normalizedPrefix) + "/{**gitPath}"; return endpoints.MapMethods(pattern, new[] { HttpMethods.Get, HttpMethods.Post }, - (HttpContext ctx) => HandleAsync(ctx, invoker, options)); + (HttpContext ctx) => HandleAsync(ctx, invoker)); } - static async Task HandleAsync(HttpContext ctx, GitHttpBackendInvoker invoker, GitBackendOptions options) + static async Task HandleAsync(HttpContext ctx, GitHttpBackendInvoker invoker) { + var options = invoker.Options; + var logger = ctx.RequestServices.GetRequiredService() .CreateLogger("GitHttpBackend.AspNetCore"); diff --git a/src/GitHttpBackend/GitHttpBackendInvoker.cs b/src/GitHttpBackend/GitHttpBackendInvoker.cs index 0326cf3..63ea2eb 100644 --- a/src/GitHttpBackend/GitHttpBackendInvoker.cs +++ b/src/GitHttpBackend/GitHttpBackendInvoker.cs @@ -44,6 +44,13 @@ public GitHttpBackendInvoker(GitBackendOptions options) /// The resolved path to the git-http-backend executable. public string BackendPath => _backendPath; + /// + /// The options this invoker was constructed with, so a host that owns the invoker does not + /// have to carry the options alongside it to reach the + /// hook. + /// + public GitBackendOptions Options => _options; + /// /// Creates the target repository when /// is set, is a push, and the repository does not exist yet. diff --git a/tests/GitHttpBackend.AspNetCore.Tests/CallerOwnedInvokerTests.cs b/tests/GitHttpBackend.AspNetCore.Tests/CallerOwnedInvokerTests.cs new file mode 100644 index 0000000..9301ad0 --- /dev/null +++ b/tests/GitHttpBackend.AspNetCore.Tests/CallerOwnedInvokerTests.cs @@ -0,0 +1,81 @@ +using System.Net; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Routing; + +namespace GitHttpBackend.AspNetCore.Tests; + +/// +/// The overload that takes an invoker the host already built. What matters is that the +/// mapping uses that instance — not a second one built from the same options — so the +/// resolved backend path the host logged is the one actually serving requests. +/// +public class CallerOwnedInvokerTests +{ + [RequiresGitFact] + public void Options_expose_what_the_invoker_was_constructed_with() + { + var options = new GitBackendOptions { ProjectRoot = Path.GetTempPath() }; + var invoker = new GitHttpBackendInvoker(options); + + Assert.Same(options, invoker.Options); + Assert.True(File.Exists(invoker.BackendPath)); + } + + [RequiresGitFact] + public async Task A_caller_owned_invoker_serves_the_repository() + { + using var git = new GitClient(); + await using var server = await GitTestServer.StartWithInvokerAsync( + root => new GitHttpBackendInvoker(new GitBackendOptions { ProjectRoot = root })); + + var work = git.CreateWorkingRepository("work"); + var bare = Path.Combine(server.ProjectRoot, "projekt.git"); + git.RunOk(server.ProjectRoot, "init", "--bare", "--", bare); + git.RunOk(work, "push", bare, "main"); + git.RunOk(bare, "symbolic-ref", "HEAD", "refs/heads/main"); + + var clone = Path.Combine(git.Root, "clone"); + git.RunOk(git.Root, "clone", new Uri(server.Client.BaseAddress!, "projekt.git").ToString(), clone); + + Assert.True(File.Exists(Path.Combine(clone, "README.md"))); + } + + [RequiresGitFact] + public async Task The_mapping_uses_the_invokers_own_options() + { + // The hook only runs if HandleAsync read the options off the instance it was handed, + // which is what stops the two overloads from drifting apart. + var authorizeCalls = 0; + await using var server = await GitTestServer.StartWithInvokerAsync( + root => new GitHttpBackendInvoker(new GitBackendOptions + { + ProjectRoot = root, + Authorize = _ => + { + Interlocked.Increment(ref authorizeCalls); + return ValueTask.FromResult(false); + }, + })); + + var response = await server.Client.GetAsync("/projekt.git/info/refs?service=git-upload-pack"); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal(1, authorizeCalls); + } + + [Fact] + public async Task Both_overloads_reject_a_null_argument() + { + await using var app = WebApplication.CreateSlimBuilder().Build(); + + Assert.Throws( + () => app.MapGitHttpBackend("/", (GitBackendOptions)null!)); + Assert.Throws( + () => app.MapGitHttpBackend("/", (GitHttpBackendInvoker)null!)); + Assert.Throws( + () => ((IEndpointRouteBuilder)null!).MapGitHttpBackend("/", new GitBackendOptions + { + ProjectRoot = Path.GetTempPath(), + })); + } +} diff --git a/tests/GitHttpBackend.AspNetCore.Tests/GitTestServer.cs b/tests/GitHttpBackend.AspNetCore.Tests/GitTestServer.cs index 25f2894..4f95762 100644 --- a/tests/GitHttpBackend.AspNetCore.Tests/GitTestServer.cs +++ b/tests/GitHttpBackend.AspNetCore.Tests/GitTestServer.cs @@ -33,7 +33,17 @@ sealed class GitTestServer : IAsyncDisposable /// Starts a server over a fresh project root. receives that /// root and returns the options to map, so a test can set its own Authorize hook. /// - public static async Task StartAsync(Func configure) + public static Task StartAsync(Func configure) + => StartCoreAsync((app, root) => app.MapGitHttpBackend("/", configure(root))); + + /// + /// Same, but the caller builds the invoker — the overload a host uses when it wants the + /// resolved backend path for itself. + /// + public static Task StartWithInvokerAsync(Func configure) + => StartCoreAsync((app, root) => app.MapGitHttpBackend("/", configure(root))); + + static async Task StartCoreAsync(Action map) { var projectRoot = Path.Combine( Path.GetTempPath(), "githttpbackend-tests", Guid.NewGuid().ToString("n")); @@ -48,7 +58,7 @@ public static async Task StartAsync(Func