diff --git a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs index e5bf026..e451f3f 100644 --- a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs +++ b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs @@ -303,6 +303,83 @@ public async Task Download_RangeRequestOnAMiss_IsStreamedWithoutStoring() Assert.AreEqual("bytes=0-3", fixture.Upstream.Requests.Last().Range); } + /// Stores an object through a whole download, then returns a fresh download href for it. + private static async Task WarmAsync(ProxyFixture fixture, byte[] content, string oid) + { + fixture.Upstream.AddObject(oid, content); + JsonNode coldBatch = await PostBatchAsync(fixture, "download", oid, content.Length); + using HttpClient client = fixture.Client; + await client.GetByteArrayAsync(Relative(HrefOf(coldBatch, "download"))); + Assert.IsTrue(fixture.Store.Exists("github", oid)); + + JsonNode warmBatch = await PostBatchAsync(fixture, "download", oid, content.Length); + return Relative(HrefOf(warmBatch, "download")); + } + + [TestMethod] + [DataRow(10L, 19L, 10, 10, DisplayName = "Closed range")] + [DataRow(null, 5L, 31, 5, DisplayName = "Suffix range")] + [DataRow(30L, null, 30, 6, DisplayName = "Open range")] + public async Task Download_RangeRequestOnAHit_ReturnsPartialContentFromTheStore( + long? from, + long? to, + int expectedStart, + int expectedLength) + { + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + (byte[] content, string oid) = Object("0123456789abcdefghijklmnopqrstuvwxyz"); + string href = await WarmAsync(fixture, content, oid); + using HttpClient client = fixture.Client; + + using HttpRequestMessage request = new(HttpMethod.Get, href); + request.Headers.Range = new System.Net.Http.Headers.RangeHeaderValue(from, to); + + using HttpResponseMessage response = await client.SendAsync(request); + + Assert.AreEqual(HttpStatusCode.PartialContent, response.StatusCode); + CollectionAssert.AreEqual( + content.Skip(expectedStart).Take(expectedLength).ToArray(), + await response.Content.ReadAsByteArrayAsync()); + Assert.AreEqual( + $"bytes {expectedStart}-{expectedStart + expectedLength - 1}/{content.Length}", + response.Content.Headers.ContentRange?.ToString()); + Assert.AreEqual(1, fixture.Upstream.FetchCount(oid), "A ranged hit must be served from the store"); + } + + [TestMethod] + public async Task Download_UnsatisfiableRangeOnAHit_Returns416() + { + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + (byte[] content, string oid) = Object("too short for that range"); + string href = await WarmAsync(fixture, content, oid); + using HttpClient client = fixture.Client; + + using HttpRequestMessage request = new(HttpMethod.Get, href); + request.Headers.Range = new System.Net.Http.Headers.RangeHeaderValue(1000, 2000); + + using HttpResponseMessage response = await client.SendAsync(request); + + Assert.AreEqual(HttpStatusCode.RequestedRangeNotSatisfiable, response.StatusCode); + Assert.AreEqual($"bytes */{content.Length}", response.Content.Headers.ContentRange?.ToString()); + } + + [TestMethod] + public async Task Download_HitWithoutARange_ReturnsTheWholeObjectAndAdvertisesRanges() + { + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + (byte[] content, string oid) = Object("whole object please"); + string href = await WarmAsync(fixture, content, oid); + using HttpClient client = fixture.Client; + + using HttpResponseMessage response = await client.GetAsync(href); + + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + CollectionAssert.AreEqual(content, await response.Content.ReadAsByteArrayAsync()); + Assert.AreEqual(content.Length, response.Content.Headers.ContentLength); + Assert.AreEqual("application/octet-stream", response.Content.Headers.ContentType?.MediaType); + CollectionAssert.Contains(response.Headers.AcceptRanges.ToList(), "bytes"); + } + [TestMethod] public async Task Download_ObjectUpstreamDoesNotHave_ReportsTheErrorPerObject() { diff --git a/GitLfsCache/Endpoints/ObjectRouteHandler.cs b/GitLfsCache/Endpoints/ObjectRouteHandler.cs index 5ad83d7..53958da 100644 --- a/GitLfsCache/Endpoints/ObjectRouteHandler.cs +++ b/GitLfsCache/Endpoints/ObjectRouteHandler.cs @@ -151,7 +151,7 @@ public async Task DownloadAsync(HttpContext context, LfsRoute route, Cancellatio metrics.RecordHit(route.Upstream, length); EndpointLog.ServedFromCache(logger, token.Oid, route.Upstream); - await ServeFromStoreAsync(context, cached, length, cancellationToken).ConfigureAwait(false); + await ServeFromStoreAsync(context, cached).ConfigureAwait(false); } return; @@ -179,8 +179,11 @@ await StreamFromUpstreamAsync(context, route, token, range, storeLocally: false, // A follower released without the object, because the leader's client went away or the // leader stalled, queues again: one of the released followers becomes the new leader and // the rest wait for it, rather than every one of them fetching the same object at once. - for (int attempt = 1; !ticket.IsLeader; attempt++) + int attempt = 0; + + while (!ticket.IsLeader) { + attempt++; EndpointLog.WaitingForLeader(logger, token.Oid, route.Upstream); bool published = await ticket @@ -198,8 +201,7 @@ await StreamFromUpstreamAsync(context, route, token, range, storeLocally: false, { store.Touch(route.Upstream, token.Oid); metrics.RecordHit(route.Upstream, nowLength); - await ServeFromStoreAsync(context, nowCached, nowLength, cancellationToken) - .ConfigureAwait(false); + await ServeFromStoreAsync(context, nowCached).ConfigureAwait(false); } return; @@ -489,19 +491,11 @@ private bool TryGetToken( return true; } - private static async Task ServeFromStoreAsync( - HttpContext context, - Stream cached, - long length, - CancellationToken cancellationToken) - { - context.Response.ContentType = OctetStream; - context.Response.ContentLength = length; - - await StreamTee - .CopyAsync(cached, context.Response.Body, null, null, cancellationToken) - .ConfigureAwait(false); - } + // ASP.NET Core's range processing answers a Range with 206 and Content-Range, an unsatisfiable + // one with 416, and advertises Accept-Ranges: bytes. That is what lets git-lfs resume an + // interrupted download of a cached object instead of starting it over. + private static Task ServeFromStoreAsync(HttpContext context, Stream cached) => + Results.Stream(cached, OctetStream, enableRangeProcessing: true).ExecuteAsync(context); private static void CopyTransferHeaders(HttpResponseMessage response, HttpContext context) {