From 9342e4f56e817775affe51e374c7692c42204233 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Thu, 12 May 2022 13:41:04 +0200 Subject: [PATCH 01/11] Add LFS API --- routers/api/v1/api.go | 1 + routers/api/v1/repo/file.go | 116 +++++++++++++++++++++++++++++++++ templates/swagger/v1_json.tmpl | 46 +++++++++++++ 3 files changed, 163 insertions(+) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 9db1d80f7f58..e6106c1a90d6 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -826,6 +826,7 @@ func Routes() *web.Route { Delete(reqAdmin(), repo.DeleteTeam) }, reqToken()) m.Get("/raw/*", context.ReferencesGitRepo(), context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetRawFile) + m.Get("/media/*", context.ReferencesGitRepo(), context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetRawFileOrLFS) m.Get("/archive/*", reqRepoReader(unit.TypeCode), repo.GetArchive) m.Combo("/forks").Get(repo.ListForks). Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 1fdf70c13a6e..126558bb39cc 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -18,7 +18,11 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/httpcache" + "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/common" @@ -75,6 +79,118 @@ func GetRawFile(ctx *context.APIContext) { } } +// GetRawFileOrLFS get a file by repo's path redirecting to LFS if necessary +func GetRawFileOrLFS(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/media/{filepath} repository repoGetRawFileOrLFS + // --- + // summary: Get a file or it's LFS object from a repository + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: filepath + // in: path + // description: filepath of the file to get + // type: string + // required: true + // - name: ref + // in: query + // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // type: string + // required: false + // responses: + // 200: + // description: Returns raw file content. + // "404": + // "$ref": "#/responses/notFound" + + if ctx.Repo.Repository.IsEmpty { + ctx.NotFound() + return + } + + blob, lastModified := getBlobForEntry(ctx) + if ctx.Written() { + return + } + + if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { + return + } + + dataRc, err := blob.DataAsync() + if err != nil { + ctx.ServerError("DataAsync", err) + return + } + closed := false + defer func() { + if closed { + return + } + if err = dataRc.Close(); err != nil { + log.Error("ServeBlobOrLFS: Close: %v", err) + } + }() + + pointer, _ := lfs.ReadPointer(dataRc) + if pointer.IsValid() { + meta, _ := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) + if meta == nil { + if err = dataRc.Close(); err != nil { + log.Error("ServeBlobOrLFS: Close: %v", err) + } + closed = true + if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { + ctx.ServerError("ServeBlob", err) + return + } + } + if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { + return + } + + if setting.LFS.ServeDirect { + // If we have a signed url (S3, object storage), redirect to this directly. + u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) + if u != nil && err == nil { + ctx.Redirect(u.String()) + return + } + } + + lfsDataRc, err := lfs.ReadMetaObject(meta.Pointer) + if err != nil { + ctx.ServerError("ReadMetaObject", err) + return + } + defer func() { + if err = lfsDataRc.Close(); err != nil { + log.Error("ServeBlobOrLFS: Close: %v", err) + } + }() + if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { + ctx.ServerError("ServeData", err) + } + return + } + if err = dataRc.Close(); err != nil { + log.Error("ServeBlobOrLFS: Close: %v", err) + } + closed = true + + if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { + ctx.ServerError("ServeBlob", err) + } +} + func getBlobForEntry(ctx *context.APIContext) (blob *git.Blob, lastModified time.Time) { entry, err := ctx.Repo.Commit.GetTreeEntryByPath(ctx.Repo.TreePath) if err != nil { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d63cde60ecf1..c23bcb2e9a93 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -7150,6 +7150,52 @@ } } }, + "/repos/{owner}/{repo}/media/{filepath}": { + "get": { + "tags": [ + "repository" + ], + "summary": "Get a file or it's LFS object from a repository", + "operationId": "repoGetRawFileOrLFS", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "filepath of the file to get", + "name": "filepath", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "in": "query" + } + ], + "responses": { + "200": { + "description": "Returns raw file content." + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/milestones": { "get": { "produces": [ From cc3a4101436c67aeecdfd9a8639ed694b2b620fc Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Sun, 15 May 2022 09:40:44 +0200 Subject: [PATCH 02/11] Update routers/api/v1/repo/file.go Co-authored-by: Gusted --- routers/api/v1/repo/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 126558bb39cc..85c8c4302430 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -79,7 +79,7 @@ func GetRawFile(ctx *context.APIContext) { } } -// GetRawFileOrLFS get a file by repo's path redirecting to LFS if necessary +// GetRawFileOrLFS get a file by repo's path, redirecting to LFS if necessary. func GetRawFileOrLFS(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/media/{filepath} repository repoGetRawFileOrLFS // --- From 562ab9f38c47dc1276d12d32f5c7b21b8be586d9 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Sun, 15 May 2022 09:53:01 +0200 Subject: [PATCH 03/11] Apply suggestions --- routers/api/v1/repo/file.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 85c8c4302430..595d3abc8efe 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -136,7 +136,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } if err = dataRc.Close(); err != nil { - log.Error("ServeBlobOrLFS: Close: %v", err) + log.Error("Close: %v", err) } }() @@ -145,13 +145,13 @@ func GetRawFileOrLFS(ctx *context.APIContext) { meta, _ := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) if meta == nil { if err = dataRc.Close(); err != nil { - log.Error("ServeBlobOrLFS: Close: %v", err) + log.Error("Close: %v", err) } closed = true if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { ctx.ServerError("ServeBlob", err) - return } + return } if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { return @@ -173,7 +173,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { } defer func() { if err = lfsDataRc.Close(); err != nil { - log.Error("ServeBlobOrLFS: Close: %v", err) + log.Error("Close: %v", err) } }() if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { @@ -182,7 +182,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } if err = dataRc.Close(); err != nil { - log.Error("ServeBlobOrLFS: Close: %v", err) + log.Error("Close: %v", err) } closed = true From 65b0876bcb63c49b9f51b5a66d079137787e4c4c Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Sun, 15 May 2022 09:56:16 +0200 Subject: [PATCH 04/11] Apply suggestions --- routers/api/v1/repo/file.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 595d3abc8efe..e93e699175db 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -130,11 +130,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { ctx.ServerError("DataAsync", err) return } - closed := false defer func() { - if closed { - return - } if err = dataRc.Close(); err != nil { log.Error("Close: %v", err) } @@ -147,7 +143,6 @@ func GetRawFileOrLFS(ctx *context.APIContext) { if err = dataRc.Close(); err != nil { log.Error("Close: %v", err) } - closed = true if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { ctx.ServerError("ServeBlob", err) } @@ -181,10 +176,6 @@ func GetRawFileOrLFS(ctx *context.APIContext) { } return } - if err = dataRc.Close(); err != nil { - log.Error("Close: %v", err) - } - closed = true if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { ctx.ServerError("ServeBlob", err) From a93847964931d880e6723c61b16211f4fc586e2f Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Sun, 15 May 2022 16:33:18 +0200 Subject: [PATCH 05/11] Update routers/api/v1/repo/file.go Co-authored-by: Gusted --- routers/api/v1/repo/file.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index e93e699175db..cee17a38eeb3 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -140,9 +140,6 @@ func GetRawFileOrLFS(ctx *context.APIContext) { if pointer.IsValid() { meta, _ := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) if meta == nil { - if err = dataRc.Close(); err != nil { - log.Error("Close: %v", err) - } if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { ctx.ServerError("ServeBlob", err) } From 13d91dcf0c69fefdd5fee1b69a6f4ce5b8c7ed8f Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Mon, 16 May 2022 16:52:56 +0200 Subject: [PATCH 06/11] Report errors --- routers/api/v1/repo/file.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index cee17a38eeb3..44c191567eae 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -138,10 +138,14 @@ func GetRawFileOrLFS(ctx *context.APIContext) { pointer, _ := lfs.ReadPointer(dataRc) if pointer.IsValid() { - meta, _ := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) - if meta == nil { - if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { - ctx.ServerError("ServeBlob", err) + meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) + if err != nil { + if err == models.ErrLFSObjectNotExist { + if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { + ctx.ServerError("ServeBlob", err) + } + } else { + ctx.ServerError("GetLFSMetaObjectByOid", err) } return } From b30ce9a293d7256c9f8508e38793568a7c7fcc75 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Sun, 22 May 2022 10:32:13 +0200 Subject: [PATCH 07/11] ADd test --- integrations/api_repo_file_get_test.go | 51 ++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 integrations/api_repo_file_get_test.go diff --git a/integrations/api_repo_file_get_test.go b/integrations/api_repo_file_get_test.go new file mode 100644 index 000000000000..cfb66ec51704 --- /dev/null +++ b/integrations/api_repo_file_get_test.go @@ -0,0 +1,51 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "net/http" + "net/url" + "os" + "testing" + + "code.gitea.io/gitea/modules/util" + "github.com/stretchr/testify/assert" +) + +func TestAPIGetRawFileOrLFS(t *testing.T) { + defer prepareTestEnv(t)() + + // Test with raw file + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/README.md") + resp := MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + + // Test with LFS + httpContext := NewAPITestContext(t, "user2", "repo1") + + onGiteaRun(t, func(t *testing.T, u *url.URL) { + u.Path = httpContext.GitPath() + dstPath, err := os.MkdirTemp("", httpContext.Reponame) + assert.NoError(t, err) + defer util.RemoveAll(dstPath) + + u.Path = httpContext.GitPath() + u.User = url.UserPassword("user2", userPassword) + + t.Run("Clone", doGitClone(dstPath, u)) + + dstPath2, err := os.MkdirTemp("", httpContext.Reponame) + assert.NoError(t, err) + defer util.RemoveAll(dstPath2) + + t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) + + lfs, _ := lfsCommitAndPushTest(t, dstPath) + + reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) + respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) + assert.Equal(t, littleSize, respLFS.Length) + }) +} From 95b746611c23d16055696df652855b03ca3c0883 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Sun, 22 May 2022 12:35:00 +0200 Subject: [PATCH 08/11] Use own repo for test --- integrations/api_repo_file_get_test.go | 38 ++++++++++++++------------ 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/integrations/api_repo_file_get_test.go b/integrations/api_repo_file_get_test.go index cfb66ec51704..9514e588363b 100644 --- a/integrations/api_repo_file_get_test.go +++ b/integrations/api_repo_file_get_test.go @@ -10,6 +10,7 @@ import ( "os" "testing" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -23,29 +24,32 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) // Test with LFS - httpContext := NewAPITestContext(t, "user2", "repo1") - onGiteaRun(t, func(t *testing.T, u *url.URL) { - u.Path = httpContext.GitPath() - dstPath, err := os.MkdirTemp("", httpContext.Reponame) - assert.NoError(t, err) - defer util.RemoveAll(dstPath) + httpContext := NewAPITestContext(t, "user2", "repo1") + doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { + u.Path = httpContext.GitPath() + dstPath, err := os.MkdirTemp("", httpContext.Reponame) + assert.NoError(t, err) + defer util.RemoveAll(dstPath) + + u.Path = httpContext.GitPath() + u.User = url.UserPassword("user2", userPassword) - u.Path = httpContext.GitPath() - u.User = url.UserPassword("user2", userPassword) + t.Run("Clone", doGitClone(dstPath, u)) - t.Run("Clone", doGitClone(dstPath, u)) + dstPath2, err := os.MkdirTemp("", httpContext.Reponame) + assert.NoError(t, err) + defer util.RemoveAll(dstPath2) - dstPath2, err := os.MkdirTemp("", httpContext.Reponame) - assert.NoError(t, err) - defer util.RemoveAll(dstPath2) + t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) + lfs, _ := lfsCommitAndPushTest(t, dstPath) - lfs, _ := lfsCommitAndPushTest(t, dstPath) + reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) + respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) + assert.Equal(t, littleSize, respLFS.Length) - reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) - respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) - assert.Equal(t, littleSize, respLFS.Length) + doAPIDeleteRepository(httpContext) + }) }) } From 97d5110d3f87595fabc114c81142fef1b6d24670 Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Sun, 22 May 2022 12:36:01 +0200 Subject: [PATCH 09/11] Use different repo name --- integrations/api_repo_file_get_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_repo_file_get_test.go b/integrations/api_repo_file_get_test.go index 9514e588363b..37105b47ff23 100644 --- a/integrations/api_repo_file_get_test.go +++ b/integrations/api_repo_file_get_test.go @@ -25,7 +25,7 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { // Test with LFS onGiteaRun(t, func(t *testing.T, u *url.URL) { - httpContext := NewAPITestContext(t, "user2", "repo1") + httpContext := NewAPITestContext(t, "user2", "repo-lfs-test") doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { u.Path = httpContext.GitPath() dstPath, err := os.MkdirTemp("", httpContext.Reponame) From 0a0b2c147ddf9cb3de6950c3f1a57d71fef5264a Mon Sep 17 00:00:00 2001 From: qwerty287 Date: Thu, 2 Jun 2022 08:54:32 +0200 Subject: [PATCH 10/11] Improve handling --- integrations/api_repo_file_get_test.go | 1 + routers/api/v1/repo/file.go | 74 +++++++++++++------------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/integrations/api_repo_file_get_test.go b/integrations/api_repo_file_get_test.go index 37105b47ff23..8d1c4c4bcf0f 100644 --- a/integrations/api_repo_file_get_test.go +++ b/integrations/api_repo_file_get_test.go @@ -12,6 +12,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" + "github.com/stretchr/testify/assert" ) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 44c191567eae..03e78a34c871 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -125,57 +125,55 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } - dataRc, err := blob.DataAsync() - if err != nil { - ctx.ServerError("DataAsync", err) - return - } - defer func() { + if blob.Size() <= 1024 { + dataRc, err := blob.DataAsync() + if err != nil { + ctx.ServerError("DataAsync", err) + return + } + + pointer, _ := lfs.ReadPointer(dataRc) if err = dataRc.Close(); err != nil { log.Error("Close: %v", err) } - }() + if pointer.IsValid() { + meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) + if err != nil { + if err == models.ErrLFSObjectNotExist { + if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { + ctx.ServerError("ServeBlob", err) + } + } else { + ctx.ServerError("GetLFSMetaObjectByOid", err) + } + return + } + if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { + return + } - pointer, _ := lfs.ReadPointer(dataRc) - if pointer.IsValid() { - meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) - if err != nil { - if err == models.ErrLFSObjectNotExist { - if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { - ctx.ServerError("ServeBlob", err) + if setting.LFS.ServeDirect { + // If we have a signed url (S3, object storage), redirect to this directly. + u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) + if u != nil && err == nil { + ctx.Redirect(u.String()) + return } - } else { - ctx.ServerError("GetLFSMetaObjectByOid", err) } - return - } - if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { - return - } - if setting.LFS.ServeDirect { - // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) - if u != nil && err == nil { - ctx.Redirect(u.String()) + lfsDataRc, err := lfs.ReadMetaObject(meta.Pointer) + if err != nil { + ctx.ServerError("ReadMetaObject", err) return } - } - - lfsDataRc, err := lfs.ReadMetaObject(meta.Pointer) - if err != nil { - ctx.ServerError("ReadMetaObject", err) - return - } - defer func() { + if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { + ctx.ServerError("ServeData", err) + } if err = lfsDataRc.Close(); err != nil { log.Error("Close: %v", err) } - }() - if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { - ctx.ServerError("ServeData", err) + return } - return } if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { From f90b2187fd0d7465c2349b4c3620afd169050949 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 3 Jun 2022 14:22:04 +0100 Subject: [PATCH 11/11] Slight restructures 1. Avoid reading the blob data multiple times 2. Ensure that caching is only checked when about to serve the blob/lfs 3. Avoid nesting by returning early 4. Make log message a bit more clear 5. Ensure that the dataRc is closed by defer when passed to ServeData Signed-off-by: Andrew Thornton --- routers/api/v1/repo/file.go | 128 +++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 46 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 03e78a34c871..ab337e66e3fa 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -6,8 +6,10 @@ package repo import ( + "bytes" "encoding/base64" "fmt" + "io" "net/http" "path" "time" @@ -121,63 +123,97 @@ func GetRawFileOrLFS(ctx *context.APIContext) { return } - if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { + // LFS Pointer files are at most 1024 bytes - so any blob greater than 1024 bytes cannot be an LFS file + if blob.Size() > 1024 { + // First handle caching for the blob + if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { + return + } + + // OK not cached - serve! + if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { + ctx.ServerError("ServeBlob", err) + } return } - if blob.Size() <= 1024 { - dataRc, err := blob.DataAsync() - if err != nil { - ctx.ServerError("DataAsync", err) + // OK, now the blob is known to have at most 1024 bytes we can simply read this in in one go (This saves reading it twice) + dataRc, err := blob.DataAsync() + if err != nil { + ctx.ServerError("DataAsync", err) + return + } + + buf, err := io.ReadAll(dataRc) + if err != nil { + _ = dataRc.Close() + ctx.ServerError("DataAsync", err) + return + } + + if err := dataRc.Close(); err != nil { + log.Error("Error whilst closing blob %s reader in %-v. Error: %v", blob.ID, ctx.Context.Repo.Repository, err) + } + + // Check if the blob represents a pointer + pointer, _ := lfs.ReadPointer(bytes.NewReader(buf)) + + // if its not a pointer just serve the data directly + if !pointer.IsValid() { + // First handle caching for the blob + if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { + return + } + + // OK not cached - serve! + if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf)); err != nil { + ctx.ServerError("ServeBlob", err) + } + return + } + + // Now check if there is a meta object for this pointer + meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) + + // If there isn't one just serve the data directly + if err == models.ErrLFSObjectNotExist { + // Handle caching for the blob SHA (not the LFS object OID) + if httpcache.HandleGenericETagTimeCache(ctx.Req, ctx.Resp, `"`+blob.ID.String()+`"`, lastModified) { return } - pointer, _ := lfs.ReadPointer(dataRc) - if err = dataRc.Close(); err != nil { - log.Error("Close: %v", err) + if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, blob.Size(), bytes.NewReader(buf)); err != nil { + ctx.ServerError("ServeBlob", err) } - if pointer.IsValid() { - meta, err := models.GetLFSMetaObjectByOid(ctx.Repo.Repository.ID, pointer.Oid) - if err != nil { - if err == models.ErrLFSObjectNotExist { - if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { - ctx.ServerError("ServeBlob", err) - } - } else { - ctx.ServerError("GetLFSMetaObjectByOid", err) - } - return - } - if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { - return - } - - if setting.LFS.ServeDirect { - // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) - if u != nil && err == nil { - ctx.Redirect(u.String()) - return - } - } - - lfsDataRc, err := lfs.ReadMetaObject(meta.Pointer) - if err != nil { - ctx.ServerError("ReadMetaObject", err) - return - } - if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { - ctx.ServerError("ServeData", err) - } - if err = lfsDataRc.Close(); err != nil { - log.Error("Close: %v", err) - } + return + } else if err != nil { + ctx.ServerError("GetLFSMetaObjectByOid", err) + return + } + + // Handle caching for the LFS object OID + if httpcache.HandleGenericETagCache(ctx.Req, ctx.Resp, `"`+pointer.Oid+`"`) { + return + } + + if setting.LFS.ServeDirect { + // If we have a signed url (S3, object storage), redirect to this directly. + u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name()) + if u != nil && err == nil { + ctx.Redirect(u.String()) return } } - if err := common.ServeBlob(ctx.Context, blob, lastModified); err != nil { - ctx.ServerError("ServeBlob", err) + lfsDataRc, err := lfs.ReadMetaObject(meta.Pointer) + if err != nil { + ctx.ServerError("ReadMetaObject", err) + return + } + defer lfsDataRc.Close() + + if err := common.ServeData(ctx.Context, ctx.Repo.TreePath, meta.Size, lfsDataRc); err != nil { + ctx.ServerError("ServeData", err) } }