diff --git a/modules/git/batch.go b/modules/git/batch.go new file mode 100644 index 0000000000..3ec4f1ddcc --- /dev/null +++ b/modules/git/batch.go @@ -0,0 +1,46 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "bufio" + "context" +) + +type Batch struct { + cancel context.CancelFunc + Reader *bufio.Reader + Writer WriteCloserError +} + +func (repo *Repository) NewBatch(ctx context.Context) (*Batch, error) { + // Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first! + if err := ensureValidGitRepository(ctx, repo.Path); err != nil { + return nil, err + } + + var batch Batch + batch.Writer, batch.Reader, batch.cancel = catFileBatch(ctx, repo.Path) + return &batch, nil +} + +func (repo *Repository) NewBatchCheck(ctx context.Context) (*Batch, error) { + // Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first! + if err := ensureValidGitRepository(ctx, repo.Path); err != nil { + return nil, err + } + + var check Batch + check.Writer, check.Reader, check.cancel = catFileBatchCheck(ctx, repo.Path) + return &check, nil +} + +func (b *Batch) Close() { + if b.cancel != nil { + b.cancel() + b.Reader = nil + b.Writer = nil + b.cancel = nil + } +} diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index c988d6ab86..3b1a466b2e 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -26,10 +26,10 @@ type WriteCloserError interface { CloseWithError(err error) error } -// EnsureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository. +// ensureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository. // Run before opening git cat-file. // This is needed otherwise the git cat-file will hang for invalid repositories. -func EnsureValidGitRepository(ctx context.Context, repoPath string) error { +func ensureValidGitRepository(ctx context.Context, repoPath string) error { stderr := strings.Builder{} err := NewCommand(ctx, "rev-parse"). SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)). @@ -43,8 +43,8 @@ func EnsureValidGitRepository(ctx context.Context, repoPath string) error { return nil } -// CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function -func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { +// catFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function +func catFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { batchStdinReader, batchStdinWriter := io.Pipe() batchStdoutReader, batchStdoutWriter := io.Pipe() ctx, ctxCancel := context.WithCancel(ctx) @@ -93,8 +93,8 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, return batchStdinWriter, batchReader, cancel } -// CatFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function -func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { +// catFileBatch opens git cat-file --batch in the provided repo and returns a stdin pipe, a stdout reader and cancel function +func catFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) { // We often want to feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary. // so let's create a batch stdin and stdout batchStdinReader, batchStdinWriter := io.Pipe() diff --git a/modules/git/blob.go b/modules/git/blob.go index 3de8ce8e90..2f02693428 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -28,9 +28,12 @@ type Blob struct { // DataAsync gets a ReadCloser for the contents of a blob without reading it all. // Calling the Close function on the result will discard all unread output. func (b *Blob) DataAsync() (io.ReadCloser, error) { - wr, rd, cancel := b.repo.CatFileBatch(b.repo.Ctx) + wr, rd, cancel, err := b.repo.CatFileBatch(b.repo.Ctx) + if err != nil { + return nil, err + } - _, err := wr.Write([]byte(b.ID.String() + "\n")) + _, err = wr.Write([]byte(b.ID.String() + "\n")) if err != nil { cancel() return nil, err @@ -66,9 +69,13 @@ func (b *Blob) Size() int64 { return b.size } - wr, rd, cancel := b.repo.CatFileBatchCheck(b.repo.Ctx) + wr, rd, cancel, err := b.repo.CatFileBatchCheck(b.repo.Ctx) + if err != nil { + log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) + return 0 + } defer cancel() - _, err := wr.Write([]byte(b.ID.String() + "\n")) + _, err = wr.Write([]byte(b.ID.String() + "\n")) if err != nil { log.Debug("error whilst reading size for %s in %s. Error: %v", b.ID.String(), b.repo.Path, err) return 0 diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index a26d749320..3b34b7933a 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -129,7 +129,10 @@ func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string, return nil, err } - batchStdinWriter, batchReader, cancel := commit.repo.CatFileBatch(ctx) + batchStdinWriter, batchReader, cancel, err := commit.repo.CatFileBatch(ctx) + if err != nil { + return nil, err + } defer cancel() commitsMap := map[string]*Commit{} diff --git a/modules/git/pipeline/lfs.go b/modules/git/pipeline/lfs.go index 55c49aaf3d..3407eb9838 100644 --- a/modules/git/pipeline/lfs.go +++ b/modules/git/pipeline/lfs.go @@ -67,7 +67,10 @@ func FindLFSFile(repo *git.Repository, objectID git.ObjectID) ([]*LFSResult, err // Next feed the commits in order into cat-file --batch, followed by their trees and sub trees as necessary. // so let's create a batch stdin and stdout - batchStdinWriter, batchReader, cancel := repo.CatFileBatch(repo.Ctx) + batchStdinWriter, batchReader, cancel, err := repo.CatFileBatch(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() // We'll use a scanner for the revList because it's simpler than a bufio.Reader diff --git a/modules/git/repo_base.go b/modules/git/repo_base.go index 8c34efc2c7..5f17bc14f6 100644 --- a/modules/git/repo_base.go +++ b/modules/git/repo_base.go @@ -21,15 +21,11 @@ type Repository struct { gpgSettings *GPGSettings - batchInUse bool - batchCancel context.CancelFunc - batchReader *bufio.Reader - batchWriter WriteCloserError + batchInUse bool + batch *Batch - checkInUse bool - checkCancel context.CancelFunc - checkReader *bufio.Reader - checkWriter WriteCloserError + checkInUse bool + check *Batch Ctx context.Context LastCommitCache *LastCommitCache @@ -51,63 +47,75 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) { return nil, errors.New("no such file or directory") } - // Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first! - if err := EnsureValidGitRepository(ctx, repoPath); err != nil { - return nil, err - } - - repo := &Repository{ + return &Repository{ Path: repoPath, tagCache: newObjectCache(), Ctx: ctx, - } - - repo.batchWriter, repo.batchReader, repo.batchCancel = CatFileBatch(ctx, repoPath) - repo.checkWriter, repo.checkReader, repo.checkCancel = CatFileBatchCheck(ctx, repoPath) - - return repo, nil + }, nil } // CatFileBatch obtains a CatFileBatch for this repository -func (repo *Repository) CatFileBatch(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) { - if repo.batchCancel == nil || repo.batchInUse { - log.Debug("Opening temporary cat file batch for: %s", repo.Path) - return CatFileBatch(ctx, repo.Path) +func (repo *Repository) CatFileBatch(ctx context.Context) (WriteCloserError, *bufio.Reader, func(), error) { + if repo.batch == nil { + var err error + repo.batch, err = repo.NewBatch(ctx) + if err != nil { + return nil, nil, nil, err + } } - repo.batchInUse = true - return repo.batchWriter, repo.batchReader, func() { - repo.batchInUse = false + + if !repo.batchInUse { + repo.batchInUse = true + return repo.batch.Writer, repo.batch.Reader, func() { + repo.batchInUse = false + }, nil } + + log.Debug("Opening temporary cat file batch for: %s", repo.Path) + tempBatch, err := repo.NewBatch(ctx) + if err != nil { + return nil, nil, nil, err + } + return tempBatch.Writer, tempBatch.Reader, tempBatch.Close, nil } // CatFileBatchCheck obtains a CatFileBatchCheck for this repository -func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) { - if repo.checkCancel == nil || repo.checkInUse { - log.Debug("Opening temporary cat file batch-check for: %s", repo.Path) - return CatFileBatchCheck(ctx, repo.Path) +func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError, *bufio.Reader, func(), error) { + if repo.check == nil { + var err error + repo.check, err = repo.NewBatchCheck(ctx) + if err != nil { + return nil, nil, nil, err + } } - repo.checkInUse = true - return repo.checkWriter, repo.checkReader, func() { - repo.checkInUse = false + + if !repo.checkInUse { + repo.checkInUse = true + return repo.check.Writer, repo.check.Reader, func() { + repo.checkInUse = false + }, nil } + + log.Debug("Opening temporary cat file batch-check for: %s", repo.Path) + tempBatchCheck, err := repo.NewBatchCheck(ctx) + if err != nil { + return nil, nil, nil, err + } + return tempBatchCheck.Writer, tempBatchCheck.Reader, tempBatchCheck.Close, nil } func (repo *Repository) Close() error { if repo == nil { return nil } - if repo.batchCancel != nil { - repo.batchCancel() - repo.batchReader = nil - repo.batchWriter = nil - repo.batchCancel = nil + if repo.batch != nil { + repo.batch.Close() + repo.batch = nil repo.batchInUse = false } - if repo.checkCancel != nil { - repo.checkCancel() - repo.checkCancel = nil - repo.checkReader = nil - repo.checkWriter = nil + if repo.check != nil { + repo.check.Close() + repo.check = nil repo.checkInUse = false } repo.LastCommitCache = nil diff --git a/modules/git/repo_base_test.go b/modules/git/repo_base_test.go new file mode 100644 index 0000000000..323b28f476 --- /dev/null +++ b/modules/git/repo_base_test.go @@ -0,0 +1,163 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package git + +import ( + "bufio" + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// This unit test relies on the implementation detail of CatFileBatch. +func TestCatFileBatch(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + repo, err := OpenRepository(ctx, "./tests/repos/repo1_bare") + require.NoError(t, err) + defer repo.Close() + + var wr WriteCloserError + var r *bufio.Reader + var cancel1 func() + t.Run("Request cat file batch", func(t *testing.T) { + assert.Nil(t, repo.batch) + wr, r, cancel1, err = repo.CatFileBatch(ctx) + require.NoError(t, err) + assert.NotNil(t, repo.batch) + assert.Equal(t, repo.batch.Writer, wr) + assert.True(t, repo.batchInUse) + }) + + t.Run("Request temporary cat file batch", func(t *testing.T) { + wr, r, cancel, err := repo.CatFileBatch(ctx) + require.NoError(t, err) + assert.NotEqual(t, repo.batch.Writer, wr) + + t.Run("Check temporary cat file batch", func(t *testing.T) { + _, err = wr.Write([]byte("95bb4d39648ee7e325106df01a621c530863a653" + "\n")) + require.NoError(t, err) + + sha, typ, size, err := ReadBatchLine(r) + require.NoError(t, err) + assert.Equal(t, "commit", typ) + assert.EqualValues(t, []byte("95bb4d39648ee7e325106df01a621c530863a653"), sha) + assert.EqualValues(t, 144, size) + }) + + cancel() + assert.True(t, repo.batchInUse) + }) + + t.Run("Check cached cat file batch", func(t *testing.T) { + _, err = wr.Write([]byte("95bb4d39648ee7e325106df01a621c530863a653" + "\n")) + require.NoError(t, err) + + sha, typ, size, err := ReadBatchLine(r) + require.NoError(t, err) + assert.Equal(t, "commit", typ) + assert.EqualValues(t, []byte("95bb4d39648ee7e325106df01a621c530863a653"), sha) + assert.EqualValues(t, 144, size) + }) + + t.Run("Cancel cached cat file batch", func(t *testing.T) { + cancel1() + assert.False(t, repo.batchInUse) + assert.NotNil(t, repo.batch) + }) + + t.Run("Request cached cat file batch", func(t *testing.T) { + wr, _, _, err := repo.CatFileBatch(ctx) + require.NoError(t, err) + assert.NotNil(t, repo.batch) + assert.Equal(t, repo.batch.Writer, wr) + assert.True(t, repo.batchInUse) + + t.Run("Close git repo", func(t *testing.T) { + require.NoError(t, repo.Close()) + assert.Nil(t, repo.batch) + }) + + _, err = wr.Write([]byte("95bb4d39648ee7e325106df01a621c530863a653" + "\n")) + require.Error(t, err) + }) +} + +// This unit test relies on the implementation detail of CatFileBatchCheck. +func TestCatFileBatchCheck(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + repo, err := OpenRepository(ctx, "./tests/repos/repo1_bare") + require.NoError(t, err) + defer repo.Close() + + var wr WriteCloserError + var r *bufio.Reader + var cancel1 func() + t.Run("Request cat file batch check", func(t *testing.T) { + assert.Nil(t, repo.check) + wr, r, cancel1, err = repo.CatFileBatchCheck(ctx) + require.NoError(t, err) + assert.NotNil(t, repo.check) + assert.Equal(t, repo.check.Writer, wr) + assert.True(t, repo.checkInUse) + }) + + t.Run("Request temporary cat file batch check", func(t *testing.T) { + wr, r, cancel, err := repo.CatFileBatchCheck(ctx) + require.NoError(t, err) + assert.NotEqual(t, repo.check.Writer, wr) + + t.Run("Check temporary cat file batch check", func(t *testing.T) { + _, err = wr.Write([]byte("test" + "\n")) + require.NoError(t, err) + + sha, typ, size, err := ReadBatchLine(r) + require.NoError(t, err) + assert.Equal(t, "tag", typ) + assert.EqualValues(t, []byte("3ad28a9149a2864384548f3d17ed7f38014c9e8a"), sha) + assert.EqualValues(t, 807, size) + }) + + cancel() + assert.True(t, repo.checkInUse) + }) + + t.Run("Check cached cat file batch check", func(t *testing.T) { + _, err = wr.Write([]byte("test" + "\n")) + require.NoError(t, err) + + sha, typ, size, err := ReadBatchLine(r) + require.NoError(t, err) + assert.Equal(t, "tag", typ) + assert.EqualValues(t, []byte("3ad28a9149a2864384548f3d17ed7f38014c9e8a"), sha) + assert.EqualValues(t, 807, size) + }) + + t.Run("Cancel cached cat file batch check", func(t *testing.T) { + cancel1() + assert.False(t, repo.checkInUse) + assert.NotNil(t, repo.check) + }) + + t.Run("Request cached cat file batch check", func(t *testing.T) { + wr, _, _, err := repo.CatFileBatchCheck(ctx) + require.NoError(t, err) + assert.NotNil(t, repo.check) + assert.Equal(t, repo.check.Writer, wr) + assert.True(t, repo.checkInUse) + + t.Run("Close git repo", func(t *testing.T) { + require.NoError(t, repo.Close()) + assert.Nil(t, repo.check) + }) + + _, err = wr.Write([]byte("test" + "\n")) + require.Error(t, err) + }) +} diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index dff3a43fa3..7339c7db0d 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -169,9 +169,13 @@ func (repo *Repository) IsObjectExist(name string) bool { return false } - wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx) + if err != nil { + log.Debug("Error writing to CatFileBatchCheck %v", err) + return false + } defer cancel() - _, err := wr.Write([]byte(name + "\n")) + _, err = wr.Write([]byte(name + "\n")) if err != nil { log.Debug("Error writing to CatFileBatchCheck %v", err) return false @@ -186,9 +190,13 @@ func (repo *Repository) IsReferenceExist(name string) bool { return false } - wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx) + if err != nil { + log.Debug("Error writing to CatFileBatchCheck %v", err) + return false + } defer cancel() - _, err := wr.Write([]byte(name + "\n")) + _, err = wr.Write([]byte(name + "\n")) if err != nil { log.Debug("Error writing to CatFileBatchCheck %v", err) return false diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 2a325d3644..1f3d64fe03 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -536,9 +536,12 @@ func (repo *Repository) ResolveReference(name string) (string, error) { // GetRefCommitID returns the last commit ID string of given reference (branch or tag). func (repo *Repository) GetRefCommitID(name string) (string, error) { - wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx) + if err != nil { + return "", err + } defer cancel() - _, err := wr.Write([]byte(name + "\n")) + _, err = wr.Write([]byte(name + "\n")) if err != nil { return "", err } @@ -564,12 +567,19 @@ func (repo *Repository) RemoveReference(name string) error { // IsCommitExist returns true if given commit exists in current repository. func (repo *Repository) IsCommitExist(name string) bool { + if err := ensureValidGitRepository(repo.Ctx, repo.Path); err != nil { + log.Error("IsCommitExist: %v", err) + return false + } _, _, err := NewCommand(repo.Ctx, "cat-file", "-e").AddDynamicArguments(name).RunStdString(&RunOpts{Dir: repo.Path}) return err == nil } func (repo *Repository) getCommit(id ObjectID) (*Commit, error) { - wr, rd, cancel := repo.CatFileBatch(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatch(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() _, _ = wr.Write([]byte(id.String() + "\n")) @@ -646,7 +656,10 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) { } } - wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() _, err = wr.Write([]byte(commitID + "\n")) if err != nil { diff --git a/modules/git/repo_language_stats.go b/modules/git/repo_language_stats.go index 84638b3cef..37c23faf68 100644 --- a/modules/git/repo_language_stats.go +++ b/modules/git/repo_language_stats.go @@ -60,7 +60,10 @@ func mergeLanguageStats(stats map[string]int64) map[string]int64 { func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, error) { // We will feed the commit IDs in order into cat-file --batch, followed by blobs as necessary. // so let's create a batch stdin and stdout - batchStdinWriter, batchReader, cancel := repo.CatFileBatch(repo.Ctx) + batchStdinWriter, batchReader, cancel, err := repo.CatFileBatch(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() writeID := func(id string) error { diff --git a/modules/git/repo_tag.go b/modules/git/repo_tag.go index d925d4a7d3..12b0c022cb 100644 --- a/modules/git/repo_tag.go +++ b/modules/git/repo_tag.go @@ -257,9 +257,12 @@ func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) { // GetTagType gets the type of the tag, either commit (simple) or tag (annotated) func (repo *Repository) GetTagType(id ObjectID) (string, error) { - wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatchCheck(repo.Ctx) + if err != nil { + return "", err + } defer cancel() - _, err := wr.Write([]byte(id.String() + "\n")) + _, err = wr.Write([]byte(id.String() + "\n")) if err != nil { return "", err } @@ -315,7 +318,10 @@ func (repo *Repository) getTag(tagID ObjectID, name string) (*Tag, error) { } // The tag is an annotated tag with a message. - wr, rd, cancel := repo.CatFileBatch(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatch(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() if _, err := wr.Write([]byte(tagID.String() + "\n")); err != nil { diff --git a/modules/git/repo_tree.go b/modules/git/repo_tree.go index 79a7e50eb4..53d94d9d7d 100644 --- a/modules/git/repo_tree.go +++ b/modules/git/repo_tree.go @@ -68,7 +68,10 @@ func (repo *Repository) CommitTree(author, committer *Signature, tree *Tree, opt } func (repo *Repository) getTree(id ObjectID) (*Tree, error) { - wr, rd, cancel := repo.CatFileBatch(repo.Ctx) + wr, rd, cancel, err := repo.CatFileBatch(repo.Ctx) + if err != nil { + return nil, err + } defer cancel() _, _ = wr.Write([]byte(id.String() + "\n")) diff --git a/modules/git/tree.go b/modules/git/tree.go index 422fe68caa..5b06cbf359 100644 --- a/modules/git/tree.go +++ b/modules/git/tree.go @@ -41,7 +41,10 @@ func (t *Tree) ListEntries() (Entries, error) { } if t.repo != nil { - wr, rd, cancel := t.repo.CatFileBatch(t.repo.Ctx) + wr, rd, cancel, err := t.repo.CatFileBatch(t.repo.Ctx) + if err != nil { + return nil, err + } defer cancel() _, _ = wr.Write([]byte(t.ID.String() + "\n")) diff --git a/modules/git/tree_entry.go b/modules/git/tree_entry.go index b5dd801309..0d9cfd2258 100644 --- a/modules/git/tree_entry.go +++ b/modules/git/tree_entry.go @@ -47,9 +47,13 @@ func (te *TreeEntry) Size() int64 { return te.size } - wr, rd, cancel := te.ptree.repo.CatFileBatchCheck(te.ptree.repo.Ctx) + wr, rd, cancel, err := te.ptree.repo.CatFileBatchCheck(te.ptree.repo.Ctx) + if err != nil { + log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), te.ptree.repo.Path, err) + return 0 + } defer cancel() - _, err := wr.Write([]byte(te.ID.String() + "\n")) + _, err = wr.Write([]byte(te.ID.String() + "\n")) if err != nil { log.Debug("error whilst reading size for %s in %s. Error: %v", te.ID.String(), te.ptree.repo.Path, err) return 0 diff --git a/modules/indexer/code/bleve/bleve.go b/modules/indexer/code/bleve/bleve.go index 66724a3445..cf9fcbd8b5 100644 --- a/modules/indexer/code/bleve/bleve.go +++ b/modules/indexer/code/bleve/bleve.go @@ -16,10 +16,10 @@ import ( "code.gitea.io/gitea/modules/analyze" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/indexer/code/internal" indexer_internal "code.gitea.io/gitea/modules/indexer/internal" inner_bleve "code.gitea.io/gitea/modules/indexer/internal/bleve" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/typesniffer" @@ -193,21 +193,23 @@ func (b *Indexer) addDelete(filename string, repo *repo_model.Repository, batch func (b *Indexer) Index(ctx context.Context, repo *repo_model.Repository, sha string, changes *internal.RepoChanges) error { batch := inner_bleve.NewFlushingBatch(b.inner.Indexer, maxBatchSize) if len(changes.Updates) > 0 { - // Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first! - if err := git.EnsureValidGitRepository(ctx, repo.RepoPath()); err != nil { - log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err) + r, err := gitrepo.OpenRepository(ctx, repo) + if err != nil { return err } - - batchWriter, batchReader, cancel := git.CatFileBatch(ctx, repo.RepoPath()) - defer cancel() + defer r.Close() + gitBatch, err := r.NewBatch(ctx) + if err != nil { + return err + } + defer gitBatch.Close() for _, update := range changes.Updates { - if err := b.addUpdate(ctx, batchWriter, batchReader, sha, update, repo, batch); err != nil { + if err := b.addUpdate(ctx, gitBatch.Writer, gitBatch.Reader, sha, update, repo, batch); err != nil { return err } } - cancel() + gitBatch.Close() } for _, filename := range changes.RemovedFilenames { if err := b.addDelete(filename, repo, batch); err != nil { diff --git a/modules/indexer/code/elasticsearch/elasticsearch.go b/modules/indexer/code/elasticsearch/elasticsearch.go index e4622fd66e..0bda180fac 100644 --- a/modules/indexer/code/elasticsearch/elasticsearch.go +++ b/modules/indexer/code/elasticsearch/elasticsearch.go @@ -15,11 +15,11 @@ import ( "code.gitea.io/gitea/modules/analyze" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/indexer/code/internal" indexer_internal "code.gitea.io/gitea/modules/indexer/internal" inner_elasticsearch "code.gitea.io/gitea/modules/indexer/internal/elasticsearch" "code.gitea.io/gitea/modules/json" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/typesniffer" @@ -154,17 +154,19 @@ func (b *Indexer) addDelete(filename string, repo *repo_model.Repository) elasti func (b *Indexer) Index(ctx context.Context, repo *repo_model.Repository, sha string, changes *internal.RepoChanges) error { reqs := make([]elastic.BulkableRequest, 0) if len(changes.Updates) > 0 { - // Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first! - if err := git.EnsureValidGitRepository(ctx, repo.RepoPath()); err != nil { - log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err) + r, err := gitrepo.OpenRepository(ctx, repo) + if err != nil { return err } - - batchWriter, batchReader, cancel := git.CatFileBatch(ctx, repo.RepoPath()) - defer cancel() + defer r.Close() + batch, err := r.NewBatch(ctx) + if err != nil { + return err + } + defer batch.Close() for _, update := range changes.Updates { - updateReqs, err := b.addUpdate(ctx, batchWriter, batchReader, sha, update, repo) + updateReqs, err := b.addUpdate(ctx, batch.Writer, batch.Reader, sha, update, repo) if err != nil { return err } @@ -172,7 +174,7 @@ func (b *Indexer) Index(ctx context.Context, repo *repo_model.Repository, sha st reqs = append(reqs, updateReqs...) } } - cancel() + batch.Close() } for _, filename := range changes.RemovedFilenames { diff --git a/routers/api/v1/repo/hook_test.go b/routers/api/v1/repo/hook_test.go index 37cf61c1ed..a8065e4a60 100644 --- a/routers/api/v1/repo/hook_test.go +++ b/routers/api/v1/repo/hook_test.go @@ -19,9 +19,11 @@ func TestTestHook(t *testing.T) { ctx, _ := contexttest.MockAPIContext(t, "user2/repo1/wiki/_pages") ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) contexttest.LoadUser(t, ctx, 2) + contexttest.LoadRepo(t, ctx, 1) + contexttest.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + contexttest.LoadRepoCommit(t, ctx) TestHook(ctx) assert.EqualValues(t, http.StatusNoContent, ctx.Resp.Status()) diff --git a/routers/web/repo/editor_test.go b/routers/web/repo/editor_test.go index 313fcfe33a..4d565b5fd6 100644 --- a/routers/web/repo/editor_test.go +++ b/routers/web/repo/editor_test.go @@ -6,6 +6,7 @@ package repo import ( "testing" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" @@ -45,7 +46,6 @@ func TestGetUniquePatchBranchName(t *testing.T) { ctx, _ := contexttest.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) contexttest.LoadUser(t, ctx, 2) contexttest.LoadGitRepo(t, ctx) defer ctx.Repo.GitRepo.Close() @@ -57,15 +57,7 @@ func TestGetUniquePatchBranchName(t *testing.T) { func TestGetClosestParentWithFiles(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) branch := repo.DefaultBranch gitRepo, _ := gitrepo.OpenRepository(git.DefaultContext, repo) defer gitRepo.Close() diff --git a/services/contexttest/context_tests.go b/services/contexttest/context_tests.go index a4cc967a57..7c829f3598 100644 --- a/services/contexttest/context_tests.go +++ b/services/contexttest/context_tests.go @@ -136,14 +136,15 @@ func LoadRepoCommit(t *testing.T, ctx gocontext.Context) { assert.FailNow(t, "context is not *context.Context or *context.APIContext") } - gitRepo, err := gitrepo.OpenRepository(ctx, repo.Repository) - require.NoError(t, err) - defer gitRepo.Close() - branch, err := gitRepo.GetHEADBranch() + if repo.GitRepo == nil { + assert.FailNow(t, "must call LoadGitRepo") + } + + branch, err := repo.GitRepo.GetHEADBranch() require.NoError(t, err) assert.NotNil(t, branch) if branch != nil { - repo.Commit, err = gitRepo.GetBranchCommit(branch.Name) + repo.Commit, err = repo.GitRepo.GetBranchCommit(branch.Name) require.NoError(t, err) } } @@ -176,10 +177,20 @@ func LoadOrganization(t *testing.T, ctx gocontext.Context, orgID int64) { // LoadGitRepo load a git repo into a test context. Requires that ctx.Repo has // already been populated. -func LoadGitRepo(t *testing.T, ctx *context.Context) { - require.NoError(t, ctx.Repo.Repository.LoadOwner(ctx)) +func LoadGitRepo(t *testing.T, ctx gocontext.Context) { + var repo *context.Repository + switch ctx := ctx.(type) { + case *context.Context: + repo = ctx.Repo + case *context.APIContext: + repo = ctx.Repo + default: + assert.FailNow(t, "context is not *context.Context or *context.APIContext") + } + + require.NoError(t, repo.Repository.LoadOwner(ctx)) var err error - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + repo.GitRepo, err = gitrepo.OpenRepository(ctx, repo.Repository) require.NoError(t, err) } diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index 768d6d2f39..c22dcd2e8d 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -6,10 +6,11 @@ package files import ( "testing" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/gitrepo" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/services/contexttest" _ "code.gitea.io/gitea/models/actions" @@ -53,27 +54,21 @@ func getExpectedReadmeContentsResponse() *api.ContentsResponse { func TestGetContents(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) treePath := "README.md" - ref := ctx.Repo.Repository.DefaultBranch + ref := repo.DefaultBranch expectedContentsResponse := getExpectedReadmeContentsResponse() t.Run("Get README.md contents with GetContents(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContents(ctx, ctx.Repo.Repository, treePath, ref, false) + fileContentResponse, err := GetContents(db.DefaultContext, repo, treePath, ref, false) assert.EqualValues(t, expectedContentsResponse, fileContentResponse) require.NoError(t, err) }) t.Run("Get README.md contents with ref as empty string (should then use the repo's default branch) with GetContents(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContents(ctx, ctx.Repo.Repository, treePath, "", false) + fileContentResponse, err := GetContents(db.DefaultContext, repo, treePath, "", false) assert.EqualValues(t, expectedContentsResponse, fileContentResponse) require.NoError(t, err) }) @@ -81,16 +76,10 @@ func TestGetContents(t *testing.T) { func TestGetContentsOrListForDir(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) treePath := "" // root dir - ref := ctx.Repo.Repository.DefaultBranch + ref := repo.DefaultBranch readmeContentsResponse := getExpectedReadmeContentsResponse() // because will be in a list, doesn't have encoding and content @@ -102,13 +91,13 @@ func TestGetContentsOrListForDir(t *testing.T) { } t.Run("Get root dir contents with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, ref) + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, treePath, ref) assert.EqualValues(t, expectedContentsListResponse, fileContentResponse) require.NoError(t, err) }) t.Run("Get root dir contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, "") + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, treePath, "") assert.EqualValues(t, expectedContentsListResponse, fileContentResponse) require.NoError(t, err) }) @@ -116,27 +105,21 @@ func TestGetContentsOrListForDir(t *testing.T) { func TestGetContentsOrListForFile(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) treePath := "README.md" - ref := ctx.Repo.Repository.DefaultBranch + ref := repo.DefaultBranch expectedContentsResponse := getExpectedReadmeContentsResponse() t.Run("Get README.md contents with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, ref) + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, treePath, ref) assert.EqualValues(t, expectedContentsResponse, fileContentResponse) require.NoError(t, err) }) t.Run("Get README.md contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList(ctx, )", func(t *testing.T) { - fileContentResponse, err := GetContentsOrList(ctx, ctx.Repo.Repository, treePath, "") + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, treePath, "") assert.EqualValues(t, expectedContentsResponse, fileContentResponse) require.NoError(t, err) }) @@ -144,28 +127,21 @@ func TestGetContentsOrListForFile(t *testing.T) { func TestGetContentsErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch t.Run("bad treePath", func(t *testing.T) { badTreePath := "bad/tree.md" - fileContentResponse, err := GetContents(ctx, repo, badTreePath, ref, false) + fileContentResponse, err := GetContents(db.DefaultContext, repo, badTreePath, ref, false) require.EqualError(t, err, "object does not exist [id: , rel_path: bad]") assert.Nil(t, fileContentResponse) }) t.Run("bad ref", func(t *testing.T) { badRef := "bad_ref" - fileContentResponse, err := GetContents(ctx, repo, treePath, badRef, false) + fileContentResponse, err := GetContents(db.DefaultContext, repo, treePath, badRef, false) require.EqualError(t, err, "object does not exist [id: "+badRef+", rel_path: ]") assert.Nil(t, fileContentResponse) }) @@ -173,28 +149,21 @@ func TestGetContentsErrors(t *testing.T) { func TestGetContentsOrListErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch t.Run("bad treePath", func(t *testing.T) { badTreePath := "bad/tree.md" - fileContentResponse, err := GetContentsOrList(ctx, repo, badTreePath, ref) + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, badTreePath, ref) require.EqualError(t, err, "object does not exist [id: , rel_path: bad]") assert.Nil(t, fileContentResponse) }) t.Run("bad ref", func(t *testing.T) { badRef := "bad_ref" - fileContentResponse, err := GetContentsOrList(ctx, repo, treePath, badRef) + fileContentResponse, err := GetContentsOrList(db.DefaultContext, repo, treePath, badRef) require.EqualError(t, err, "object does not exist [id: "+badRef+", rel_path: ]") assert.Nil(t, fileContentResponse) }) @@ -202,17 +171,10 @@ func TestGetContentsOrListErrors(t *testing.T) { func TestGetContentsOrListOfEmptyRepos(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user30/empty") - ctx.SetParams(":id", "52") - contexttest.LoadRepo(t, ctx, 52) - contexttest.LoadUser(t, ctx, 30) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 52}) t.Run("empty repo", func(t *testing.T) { - contents, err := GetContentsOrList(ctx, repo, "", "") + contents, err := GetContentsOrList(db.DefaultContext, repo, "", "") require.NoError(t, err) assert.Empty(t, contents) }) @@ -220,23 +182,13 @@ func TestGetContentsOrListOfEmptyRepos(t *testing.T) { func TestGetBlobBySHA(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" - ctx.SetParams(":id", "1") - ctx.SetParams(":sha", sha) + gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) + require.NoError(t, err) + defer gitRepo.Close() - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) - if err != nil { - t.Fail() - } - - gbr, err := GetBlobBySHA(ctx, ctx.Repo.Repository, gitRepo, ctx.Params(":sha")) + gbr, err := GetBlobBySHA(db.DefaultContext, repo, gitRepo, "65f1bf27bc3bf70f64657658635e66094edbcb4d") expectedGBR := &api.GitBlobResponse{ Content: "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK", Encoding: "base64", diff --git a/services/repository/files/diff_test.go b/services/repository/files/diff_test.go index 1ea4a170cc..95de10e07e 100644 --- a/services/repository/files/diff_test.go +++ b/services/repository/files/diff_test.go @@ -6,6 +6,7 @@ package files import ( "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/json" @@ -21,7 +22,6 @@ func TestGetDiffPreview(t *testing.T) { ctx, _ := contexttest.MockContext(t, "user2/repo1") ctx.SetParams(":id", "1") contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) contexttest.LoadUser(t, ctx, 2) contexttest.LoadGitRepo(t, ctx) defer ctx.Repo.GitRepo.Close() @@ -140,33 +140,26 @@ func TestGetDiffPreview(t *testing.T) { func TestGetDiffPreviewErrors(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - branch := ctx.Repo.Repository.DefaultBranch + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + branch := repo.DefaultBranch treePath := "README.md" content := "# repo1\n\nDescription for repo1\nthis is a new line" t.Run("empty repo", func(t *testing.T) { - diff, err := GetDiffPreview(ctx, &repo_model.Repository{}, branch, treePath, content) + diff, err := GetDiffPreview(db.DefaultContext, &repo_model.Repository{}, branch, treePath, content) assert.Nil(t, diff) assert.EqualError(t, err, "repository does not exist [id: 0, uid: 0, owner_name: , name: ]") }) t.Run("bad branch", func(t *testing.T) { badBranch := "bad_branch" - diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, badBranch, treePath, content) + diff, err := GetDiffPreview(db.DefaultContext, repo, badBranch, treePath, content) assert.Nil(t, diff) assert.EqualError(t, err, "branch does not exist [name: "+badBranch+"]") }) t.Run("empty treePath", func(t *testing.T) { - diff, err := GetDiffPreview(ctx, ctx.Repo.Repository, branch, "", content) + diff, err := GetDiffPreview(db.DefaultContext, repo, branch, "", content) assert.Nil(t, diff) assert.EqualError(t, err, "path is invalid [path: ]") }) diff --git a/services/repository/files/file_test.go b/services/repository/files/file_test.go index 2c6a169da1..7c387e2dd5 100644 --- a/services/repository/files/file_test.go +++ b/services/repository/files/file_test.go @@ -6,11 +6,12 @@ package files import ( "testing" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/services/contexttest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -99,23 +100,16 @@ func getExpectedFileResponse() *api.FileResponse { func TestGetFileResponseFromCommit(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - repo := ctx.Repo.Repository + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) branch := repo.DefaultBranch treePath := "README.md" - gitRepo, _ := gitrepo.OpenRepository(ctx, repo) + gitRepo, _ := gitrepo.OpenRepository(db.DefaultContext, repo) defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(branch) expectedFileResponse := getExpectedFileResponse() - fileResponse, err := GetFileResponseFromCommit(ctx, repo, commit, branch, treePath) + fileResponse, err := GetFileResponseFromCommit(db.DefaultContext, repo, commit, branch, treePath) require.NoError(t, err) assert.EqualValues(t, expectedFileResponse, fileResponse) } diff --git a/services/repository/files/tree_test.go b/services/repository/files/tree_test.go index faa9b8e29e..9e5c5c1701 100644 --- a/services/repository/files/tree_test.go +++ b/services/repository/files/tree_test.go @@ -18,7 +18,6 @@ func TestGetTreeBySHA(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "user2/repo1") contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) contexttest.LoadUser(t, ctx, 2) contexttest.LoadGitRepo(t, ctx) defer ctx.Repo.GitRepo.Close() diff --git a/tests/integration/repo_view_test.go b/tests/integration/repo_view_test.go index 290686d554..7c280e2491 100644 --- a/tests/integration/repo_view_test.go +++ b/tests/integration/repo_view_test.go @@ -52,8 +52,13 @@ func createRepoAndGetContext(t *testing.T, files []string, deleteMdReadme bool) ctx, _ := contexttest.MockContext(t, "user1/readmetest") ctx.SetParams(":id", fmt.Sprint(repo.ID)) contexttest.LoadRepo(t, ctx, repo.ID) + contexttest.LoadGitRepo(t, ctx) contexttest.LoadRepoCommit(t, ctx) - return ctx, f + + return ctx, func() { + f() + ctx.Repo.GitRepo.Close() + } } func TestRepoView_FindReadme(t *testing.T) { diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index 7f5e17c2ce..9790b36183 100644 --- a/tests/integration/repofiles_change_test.go +++ b/tests/integration/repofiles_change_test.go @@ -12,11 +12,11 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "code.gitea.io/gitea/services/contexttest" files_service "code.gitea.io/gitea/services/repository/files" "github.com/stretchr/testify/assert" @@ -247,16 +247,8 @@ func getExpectedFileResponseForRepofilesUpdate(commitID, filename, lastCommitSHA func TestChangeRepoFilesForCreate(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) opts := getCreateRepoFilesOptions(repo) // test @@ -284,16 +276,8 @@ func TestChangeRepoFilesForCreate(t *testing.T) { func TestChangeRepoFilesForUpdate(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) opts := getUpdateRepoFilesOptions(repo) // test @@ -318,16 +302,8 @@ func TestChangeRepoFilesForUpdate(t *testing.T) { func TestChangeRepoFilesForUpdateWithFileMove(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) opts := getUpdateRepoFilesOptions(repo) opts.Files[0].FromTreePath = "README.md" opts.Files[0].TreePath = "README_new.md" // new file name, README_new.md @@ -369,16 +345,8 @@ func TestChangeRepoFilesForUpdateWithFileMove(t *testing.T) { func TestChangeRepoFilesWithoutBranchNames(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) opts := getUpdateRepoFilesOptions(repo) opts.OldBranch = "" opts.NewBranch = "" @@ -405,15 +373,8 @@ func TestChangeRepoFilesForDelete(t *testing.T) { func testDeleteRepoFiles(t *testing.T, u *url.URL) { // setup unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) opts := getDeleteRepoFilesOptions(repo) t.Run("Delete README.md file", func(t *testing.T) { @@ -444,16 +405,9 @@ func TestChangeRepoFilesForDeleteWithoutBranchNames(t *testing.T) { func testDeleteRepoFilesWithoutBranchNames(t *testing.T, u *url.URL) { // setup unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repo := ctx.Repo.Repository - doer := ctx.Doer opts := getDeleteRepoFilesOptions(repo) opts.OldBranch = "" opts.NewBranch = "" @@ -474,16 +428,8 @@ func testDeleteRepoFilesWithoutBranchNames(t *testing.T, u *url.URL) { func TestChangeRepoFilesErrors(t *testing.T) { // setup onGiteaRun(t, func(t *testing.T, u *url.URL) { - ctx, _ := contexttest.MockContext(t, "user2/repo1") - ctx.SetParams(":id", "1") - contexttest.LoadRepo(t, ctx, 1) - contexttest.LoadRepoCommit(t, ctx) - contexttest.LoadUser(t, ctx, 2) - contexttest.LoadGitRepo(t, ctx) - defer ctx.Repo.GitRepo.Close() - - repo := ctx.Repo.Repository - doer := ctx.Doer + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) t.Run("bad branch", func(t *testing.T) { opts := getUpdateRepoFilesOptions(repo)