artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 14:51:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

gitserver: Unify filereader API (#59782)

Browse Source 
This PR consolidates two methods that were effectively doing the same thing: `ReadFile` and `NewFileReader` - where `ReadFile` used the latter under the hood, and wrapped an `io.ReadAll`.
Given entire files are returned from this function that can be of arbitrary size, exposing a simple high-memory-use footgun made me uneasy here, and having to use io.ReadAll explicitly as the consumer hopefully makes the person using this API feel the same way.
Sometimes, we can currently not avoid it (GQL layer), but where possible, using the reader is very likely the better option. 

Since there would've been a slight change in how errors are reported, I also fixed that. Previously, `NewFileReader` would NOT return `os.ErrNotExist` errors, those would only be discovered when the returned reader is called.
Now, like `ReadFile`, and like `os.Open` we immediately return that error.

This comes at the cost of making a second gRPC request. This is not great, but we will migrate the FileReader API to a proper gRPC hopefully ASAP, which will let us optimize it better on the server-side.
Since Varun also asked for perf improvements here, this is probably one of the first things that would be easy to switch over to go-git, eliminating probably 50%+ of the overhead here.
This commit is contained in:
Erik Seliger 2024-01-24 13:53:57 +01:00 committed by GitHub
parent f50d804f37
commit f999dd3420
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
35 changed files with 303 additions and 554 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cmd/frontend/graphqlbackend/git_tree_entry.go
View File

@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"io"
"io/fs"
"net/url"
"os"
@ -110,12 +111,19 @@ func (r *GitTreeEntryResolver) ByteSize(ctx context.Context) (int32, error) {
func (r *GitTreeEntryResolver) Content(ctx context.Context, args *GitTreeContentPageArgs) (string, error) {
r.contentOnce.Do(func() {
r.fullContentBytes, r.contentErr = r.gitserverClient.ReadFile(
fr, err := r.gitserverClient.NewFileReader(
ctx,
r.commit.repoResolver.RepoName(),
api.CommitID(r.commit.OID()),
r.Path(),
)
if err != nil {
r.contentErr = err
return
}
defer fr.Close()
r.fullContentBytes, r.contentErr = io.ReadAll(fr)
})
return string(pageContent(r.fullContentBytes, int32ToIntPtr(args.StartLine), int32ToIntPtr(args.EndLine))), r.contentErr

10
cmd/frontend/graphqlbackend/git_tree_entry_test.go
View File

@ -1,7 +1,9 @@
package graphqlbackend
import (
"bytes"
"context"
"io"
"testing"
"github.com/google/go-cmp/cmp"
@ -37,11 +39,11 @@ func TestGitTreeEntry_Content(t *testing.T) {
db := dbmocks.NewMockDB()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, name string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, name string) (io.ReadCloser, error) {
if name != wantPath {
t.Fatalf("wrong name in ReadFile call. want=%q, have=%q", wantPath, name)
}
return []byte(wantContent), nil
return io.NopCloser(bytes.NewReader([]byte(wantContent))), nil
})
opts := GitTreeEntryResolverOpts{
Commit: &GitCommitResolver{
@ -76,11 +78,11 @@ func TestGitTreeEntry_ContentPagination(t *testing.T) {
db := dbmocks.NewMockDB()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, name string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, name string) (io.ReadCloser, error) {
if name != wantPath {
t.Fatalf("wrong name in ReadFile call. want=%q, have=%q", wantPath, name)
}
return testContent, nil
return io.NopCloser(bytes.NewReader(testContent)), nil
})
tests := []struct {

6
cmd/frontend/graphqlbackend/preview_repository_comparison_test.go
View File

@ -1,7 +1,9 @@
package graphqlbackend
import (
"bytes"
"context"
"io"
"testing"
"time"
@ -239,11 +241,11 @@ index 9bd8209..d2acfa9 100644
}
fileDiff := fileDiffs[0]
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, name string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, name string) (io.ReadCloser, error) {
if name != "INSTALL.md" {
t.Fatalf("ReadFile received call for wrong file: %s", name)
}
return []byte(testOldFile), nil
return io.NopCloser(bytes.NewReader([]byte(testOldFile))), nil
})
newFile := fileDiff.NewFile()

6
cmd/frontend/internal/context/resolvers/context_test.go
View File

@ -1,7 +1,9 @@
package resolvers
import (
"bytes"
"context"
"io"
"io/fs"
"os"
"sort"
@ -121,9 +123,9 @@ func TestContextResolver(t *testing.T) {
mockGitserver.StatFunc.SetDefaultHook(func(_ context.Context, repo api.RepoName, _ api.CommitID, fileName string) (fs.FileInfo, error) {
return fakeFileInfo{path: fileName}, nil
})
mockGitserver.ReadFileFunc.SetDefaultHook(func(_ context.Context, repo api.RepoName, _ api.CommitID, fileName string) ([]byte, error) {
mockGitserver.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName, ci api.CommitID, fileName string) (io.ReadCloser, error) {
if content, ok := files[repo][fileName]; ok {
return content, nil
return io.NopCloser(bytes.NewReader(content)), nil
}
return nil, os.ErrNotExist
})

10
cmd/frontend/internal/embeddings/resolvers/resolvers.go
View File

@ -3,6 +3,7 @@ package resolvers
import (
"bytes"
"context"
"io"
"os"
"github.com/graph-gophers/graphql-go"
@ -219,7 +220,14 @@ func embeddingsSearchResultsToResolvers(
for i, result := range results {
i, result := i, result
p.Go(func() {
content, err := gs.ReadFile(ctx, result.RepoName, result.Revision, result.FileName)
r, err := gs.NewFileReader(ctx, result.RepoName, result.Revision, result.FileName)
if err != nil {
allContents[i] = nil
allErrors[i] = err
return
}
defer r.Close()
content, err := io.ReadAll(r)
allContents[i] = content
allErrors[i] = err
})

6
cmd/frontend/internal/embeddings/resolvers/resolvers_test.go
View File

@ -1,7 +1,9 @@
package resolvers
import (
"bytes"
"context"
"io"
"os"
"testing"
"time"
@ -70,9 +72,9 @@ func TestEmbeddingSearchResolver(t *testing.T) {
mockDB.PermissionsFunc.SetDefaultReturn(permissions)
mockGitserver := gitserver.NewMockClient()
mockGitserver.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, fileName string) ([]byte, error) {
mockGitserver.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, fileName string) (io.ReadCloser, error) {
if fileName == "testfile" {
return []byte("test\nfirst\nfour\nlines\nplus\nsome\nmore"), nil
return io.NopCloser(bytes.NewReader([]byte("test\nfirst\nfour\nlines\nplus\nsome\nmore"))), nil
}
return nil, os.ErrNotExist
})

6
cmd/frontend/internal/own/resolvers/resolvers_test.go
View File

@ -1,9 +1,11 @@
package resolvers_test
import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
"strings"
@ -109,7 +111,7 @@ func fakeOwnDb() *dbmocks.MockDB {
type repoFiles map[repoPath]string
func (g fakeGitserver) ReadFile(_ context.Context, repoName api.RepoName, commitID api.CommitID, file string) ([]byte, error) {
func (g fakeGitserver) NewFileReader(_ context.Context, repoName api.RepoName, commitID api.CommitID, file string) (io.ReadCloser, error) {
if g.files == nil {
return nil, os.ErrNotExist
}
@ -117,7 +119,7 @@ func (g fakeGitserver) ReadFile(_ context.Context, repoName api.RepoName, commit
if !ok {
return nil, os.ErrNotExist
}
return []byte(content), nil
return io.NopCloser(bytes.NewReader([]byte(content))), nil
}
// Stat is a fake implementation that returns a FileInfo

26
cmd/gitserver/internal/integration_tests/tree_test.go
View File

@ -3,6 +3,7 @@ package inttests
import (
"bytes"
"context"
"io"
"io/fs"
"os"
"path/filepath"
@ -11,6 +12,7 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/api"
@ -120,11 +122,15 @@ func TestRepository_FileSystem(t *testing.T) {
}
// dir1/file1 should exist, contain "infile1", have the right mtime, and be a file.
file1Data, err := client.ReadFile(ctx, test.repo, test.first, "dir1/file1")
file1R, err := client.NewFileReader(ctx, test.repo, test.first, "dir1/file1")
if err != nil {
t.Errorf("%s: fs1.ReadFile(dir1/file1): %s", label, err)
continue
}
file1Data, err := io.ReadAll(file1R)
file1R.Close()
require.NoError(t, err)
if !bytes.Equal(file1Data, []byte("infile1")) {
t.Errorf("%s: got file1Data == %q, want %q", label, string(file1Data), "infile1")
}
@ -144,27 +150,34 @@ func TestRepository_FileSystem(t *testing.T) {
}
// file 2 shouldn't exist in the 1st commit.
_, err = client.ReadFile(ctx, test.repo, test.first, "file 2")
_, err = client.NewFileReader(ctx, test.repo, test.first, "file 2")
if !os.IsNotExist(err) {
t.Errorf("%s: fs1.Open(file 2): got err %v, want os.IsNotExist (file 2 should not exist in this commit)", label, err)
}
// file 2 should exist in the 2nd commit.
_, err = client.ReadFile(ctx, test.repo, test.second, "file 2")
file2R, err := client.NewFileReader(ctx, test.repo, test.second, "file 2")
if err != nil {
t.Errorf("%s: fs2.Open(file 2): %s", label, err)
continue
}
_, err = io.ReadAll(file2R)
file2R.Close()
require.NoError(t, err)
// file1 should also exist in the 2nd commit.
if _, err := client.Stat(ctx, test.repo, test.second, "dir1/file1"); err != nil {
t.Errorf("%s: fs2.Stat(dir1/file1): %s", label, err)
continue
}
if _, err := client.ReadFile(ctx, test.repo, test.second, "dir1/file1"); err != nil {
file1R, err = client.NewFileReader(ctx, test.repo, test.second, "dir1/file1")
if err != nil {
t.Errorf("%s: fs2.Open(dir1/file1): %s", label, err)
continue
}
_, err = io.ReadAll(file1R)
file1R.Close()
require.NoError(t, err)
// root should exist (via Stat).
root, err := client.Stat(ctx, test.repo, test.second, ".")
@ -367,11 +380,14 @@ func TestRepository_FileSystem_gitSubmodules(t *testing.T) {
// .gitmodules file is entries[0]
checkSubmoduleFileInfo(label+" (ReadDir)", entries[1])
_, err = client.ReadFile(ctx, test.repo, commitID, "submod")
r, err := client.NewFileReader(ctx, test.repo, commitID, "submod")
if err != nil {
t.Errorf("%s: fs.Open(submod): %s", label, err)
continue
}
_, err = io.ReadAll(r)
r.Close()
require.NoError(t, err)
}
}

12
cmd/searcher/internal/search/filter.go
View File

@ -2,10 +2,10 @@ package search
import (
"archive/tar"
"bytes"
"context"
"hash"
"io"
"os"
"strings"
"github.com/bmatcuk/doublestar"
@ -19,18 +19,20 @@ import (
// NewFilter calls gitserver to retrieve the ignore-file. If the file doesn't
// exist we return an empty ignore.Matcher.
func NewFilter(ctx context.Context, client gitserver.Client, repo api.RepoName, commit api.CommitID) (FilterFunc, error) {
ignoreFile, err := client.ReadFile(ctx, repo, commit, ignore.IgnoreFile)
r, err := client.NewFileReader(ctx, repo, commit, ignore.IgnoreFile)
if err != nil {
// We do not ignore anything if the ignore file does not exist.
if strings.Contains(err.Error(), "file does not exist") {
if os.IsNotExist(err) {
// We do not ignore anything if the ignore file does not exist.
return func(*tar.Header) bool {
return false
}, nil
}
return nil, err
}
defer r.Close()
ig, err := ignore.ParseIgnoreFile(bytes.NewReader(ignoreFile))
ig, err := ignore.ParseIgnoreFile(r)
if err != nil {
return nil, err
}

8
cmd/searcher/internal/search/filter_test.go
View File

@ -2,17 +2,19 @@ package search
import (
"archive/tar"
"bytes"
"context"
"io"
"os"
"testing"
"testing/quick"
"github.com/sourcegraph/sourcegraph/internal/gitserver"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
func TestNewFilter(t *testing.T) {
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultReturn([]byte("foo/"), nil)
gitserverClient.NewFileReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte("foo/"))), nil)
ig, err := NewFilter(context.Background(), gitserverClient, "", "")
if err != nil {
@ -51,7 +53,7 @@ func TestNewFilter(t *testing.T) {
func TestMissingIgnoreFile(t *testing.T) {
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultReturn(nil, errors.Errorf("err open .sourcegraph/ignore: file does not exist"))
gitserverClient.NewFileReaderFunc.SetDefaultReturn(nil, os.ErrNotExist)
ig, err := NewFilter(context.Background(), gitserverClient, "", "")
if err != nil {

91
cmd/symbols/fetcher/mocks_test.go generated
View File

@ -31,9 +31,9 @@ type MockGitserverClient struct {
// LogReverseEachFunc is an instance of a mock function object
// controlling the behavior of the method LogReverseEach.
LogReverseEachFunc *GitserverClientLogReverseEachFunc
// ReadFileFunc is an instance of a mock function object controlling the
// behavior of the method ReadFile.
ReadFileFunc *GitserverClientReadFileFunc
// NewFileReaderFunc is an instance of a mock function object
// controlling the behavior of the method NewFileReader.
NewFileReaderFunc *GitserverClientNewFileReaderFunc
// RevListFunc is an instance of a mock function object controlling the
// behavior of the method RevList.
RevListFunc *GitserverClientRevListFunc
@ -59,8 +59,8 @@ func NewMockGitserverClient() *MockGitserverClient {
return
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (r0 []byte, r1 error) {
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (r0 io.ReadCloser, r1 error) {
return
},
},
@ -91,9 +91,9 @@ func NewStrictMockGitserverClient() *MockGitserverClient {
panic("unexpected invocation of MockGitserverClient.LogReverseEach")
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, types.RepoCommitPath) ([]byte, error) {
panic("unexpected invocation of MockGitserverClient.ReadFile")
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
panic("unexpected invocation of MockGitserverClient.NewFileReader")
},
},
RevListFunc: &GitserverClientRevListFunc{
@ -118,8 +118,8 @@ func NewMockGitserverClientFrom(i gitserver.GitserverClient) *MockGitserverClien
LogReverseEachFunc: &GitserverClientLogReverseEachFunc{
defaultHook: i.LogReverseEach,
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: i.ReadFile,
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: i.NewFileReader,
},
RevListFunc: &GitserverClientRevListFunc{
defaultHook: i.RevList,
@ -471,35 +471,36 @@ func (c GitserverClientLogReverseEachFuncCall) Results() []interface{} {
return []interface{}{c.Result0}
}
// GitserverClientReadFileFunc describes the behavior when the ReadFile
// method of the parent MockGitserverClient instance is invoked.
type GitserverClientReadFileFunc struct {
defaultHook func(context.Context, types.RepoCommitPath) ([]byte, error)
hooks []func(context.Context, types.RepoCommitPath) ([]byte, error)
history []GitserverClientReadFileFuncCall
// GitserverClientNewFileReaderFunc describes the behavior when the
// NewFileReader method of the parent MockGitserverClient instance is
// invoked.
type GitserverClientNewFileReaderFunc struct {
defaultHook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)
hooks []func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)
history []GitserverClientNewFileReaderFuncCall
mutex sync.Mutex
}
// ReadFile delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockGitserverClient) ReadFile(v0 context.Context, v1 types.RepoCommitPath) ([]byte, error) {
r0, r1 := m.ReadFileFunc.nextHook()(v0, v1)
m.ReadFileFunc.appendCall(GitserverClientReadFileFuncCall{v0, v1, r0, r1})
// NewFileReader delegates to the next hook function in the queue and stores
// the parameter and result values of this invocation.
func (m *MockGitserverClient) NewFileReader(v0 context.Context, v1 types.RepoCommitPath) (io.ReadCloser, error) {
r0, r1 := m.NewFileReaderFunc.nextHook()(v0, v1)
m.NewFileReaderFunc.appendCall(GitserverClientNewFileReaderFuncCall{v0, v1, r0, r1})
return r0, r1
}
// SetDefaultHook sets function that is called when the ReadFile method of
// the parent MockGitserverClient instance is invoked and the hook queue is
// empty.
func (f *GitserverClientReadFileFunc) SetDefaultHook(hook func(context.Context, types.RepoCommitPath) ([]byte, error)) {
// SetDefaultHook sets function that is called when the NewFileReader method
// of the parent MockGitserverClient instance is invoked and the hook queue
// is empty.
func (f *GitserverClientNewFileReaderFunc) SetDefaultHook(hook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// ReadFile method of the parent MockGitserverClient instance invokes the
// hook at the front of the queue and discards it. After the queue is empty,
// the default hook function is invoked for any future action.
func (f *GitserverClientReadFileFunc) PushHook(hook func(context.Context, types.RepoCommitPath) ([]byte, error)) {
// NewFileReader method of the parent MockGitserverClient instance invokes
// the hook at the front of the queue and discards it. After the queue is
// empty, the default hook function is invoked for any future action.
func (f *GitserverClientNewFileReaderFunc) PushHook(hook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
@ -507,20 +508,20 @@ func (f *GitserverClientReadFileFunc) PushHook(hook func(context.Context, types.
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *GitserverClientReadFileFunc) SetDefaultReturn(r0 []byte, r1 error) {
f.SetDefaultHook(func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) SetDefaultReturn(r0 io.ReadCloser, r1 error) {
f.SetDefaultHook(func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
return r0, r1
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *GitserverClientReadFileFunc) PushReturn(r0 []byte, r1 error) {
f.PushHook(func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) PushReturn(r0 io.ReadCloser, r1 error) {
f.PushHook(func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
return r0, r1
})
}
func (f *GitserverClientReadFileFunc) nextHook() func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) nextHook() func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
@ -533,26 +534,26 @@ func (f *GitserverClientReadFileFunc) nextHook() func(context.Context, types.Rep
return hook
}
func (f *GitserverClientReadFileFunc) appendCall(r0 GitserverClientReadFileFuncCall) {
func (f *GitserverClientNewFileReaderFunc) appendCall(r0 GitserverClientNewFileReaderFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of GitserverClientReadFileFuncCall objects
// describing the invocations of this function.
func (f *GitserverClientReadFileFunc) History() []GitserverClientReadFileFuncCall {
// History returns a sequence of GitserverClientNewFileReaderFuncCall
// objects describing the invocations of this function.
func (f *GitserverClientNewFileReaderFunc) History() []GitserverClientNewFileReaderFuncCall {
f.mutex.Lock()
history := make([]GitserverClientReadFileFuncCall, len(f.history))
history := make([]GitserverClientNewFileReaderFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// GitserverClientReadFileFuncCall is an object that describes an invocation
// of method ReadFile on an instance of MockGitserverClient.
type GitserverClientReadFileFuncCall struct {
// GitserverClientNewFileReaderFuncCall is an object that describes an
// invocation of method NewFileReader on an instance of MockGitserverClient.
type GitserverClientNewFileReaderFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
@ -561,7 +562,7 @@ type GitserverClientReadFileFuncCall struct {
Arg1 types.RepoCommitPath
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 []byte
Result0 io.ReadCloser
// Result1 is the value of the 2nd result returned from this method
// invocation.
Result1 error
@ -569,13 +570,13 @@ type GitserverClientReadFileFuncCall struct {
// Args returns an interface slice containing the arguments of this
// invocation.
func (c GitserverClientReadFileFuncCall) Args() []interface{} {
func (c GitserverClientNewFileReaderFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c GitserverClientReadFileFuncCall) Results() []interface{} {
func (c GitserverClientNewFileReaderFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}

14
cmd/symbols/gitserver/client.go
View File

@ -25,8 +25,10 @@ type GitserverClient interface {
// GitDiff returns the paths that have changed between two commits.
GitDiff(context.Context, api.RepoName, api.CommitID, api.CommitID) (Changes, error)
// ReadFile returns the file content for the given file at a repo commit.
ReadFile(ctx context.Context, repoCommitPath types.RepoCommitPath) ([]byte, error)
// NewFileReader returns an io.ReadCloser reading from the named file at commit.
// The caller should always close the reader after use.
// (If you just need to check a file's existence, use Stat, not a file reader.)
NewFileReader(ctx context.Context, repoCommitPath types.RepoCommitPath) (io.ReadCloser, error)
// LogReverseEach runs git log in reverse order and calls the given callback for each entry.
LogReverseEach(ctx context.Context, repo string, commit string, n int, onLogEntry func(entry gitdomain.LogEntry) error) error
@ -96,12 +98,8 @@ func (c *gitserverClient) GitDiff(ctx context.Context, repo api.RepoName, commit
return changes, nil
}
func (c *gitserverClient) ReadFile(ctx context.Context, repoCommitPath types.RepoCommitPath) ([]byte, error) {
data, err := c.innerClient.ReadFile(ctx, api.RepoName(repoCommitPath.Repo), api.CommitID(repoCommitPath.Commit), repoCommitPath.Path)
if err != nil {
return nil, errors.Wrap(err, "failed to get file contents")
}
return data, nil
func (c *gitserverClient) NewFileReader(ctx context.Context, repoCommitPath types.RepoCommitPath) (io.ReadCloser, error) {
return c.innerClient.NewFileReader(ctx, api.RepoName(repoCommitPath.Repo), api.CommitID(repoCommitPath.Commit), repoCommitPath.Path)
}
func (c *gitserverClient) LogReverseEach(ctx context.Context, repo string, commit string, n int, onLogEntry func(entry gitdomain.LogEntry) error) error {

3
cmd/symbols/internal/api/handler_test.go
View File

@ -26,6 +26,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/search"
"github.com/sourcegraph/sourcegraph/internal/search/result"
symbolsclient "github.com/sourcegraph/sourcegraph/internal/symbols"
types "github.com/sourcegraph/sourcegraph/internal/types"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
@ -69,7 +70,7 @@ func TestHandler(t *testing.T) {
symbolParser := parser.NewParser(&observation.TestContext, parserPool, fetcher.NewRepositoryFetcher(&observation.TestContext, gitserverClient, 1000, 1_000_000), 0, 10)
databaseWriter := writer.NewDatabaseWriter(observation.TestContextTB(t), tmpDir, gitserverClient, symbolParser, semaphore.NewWeighted(1))
cachedDatabaseWriter := writer.NewCachedDatabaseWriter(databaseWriter, cache)
handler := NewHandler(MakeSqliteSearchFunc(observation.TestContextTB(t), cachedDatabaseWriter, dbmocks.NewMockDB()), gitserverClient.ReadFile, nil, "")
handler := NewHandler(MakeSqliteSearchFunc(observation.TestContextTB(t), cachedDatabaseWriter, dbmocks.NewMockDB()), func(ctx context.Context, rcp types.RepoCommitPath) ([]byte, error) { return nil, nil }, nil, "")
server := httptest.NewServer(handler)
defer server.Close()

91
cmd/symbols/internal/api/mocks_test.go generated
View File

@ -31,9 +31,9 @@ type MockGitserverClient struct {
// LogReverseEachFunc is an instance of a mock function object
// controlling the behavior of the method LogReverseEach.
LogReverseEachFunc *GitserverClientLogReverseEachFunc
// ReadFileFunc is an instance of a mock function object controlling the
// behavior of the method ReadFile.
ReadFileFunc *GitserverClientReadFileFunc
// NewFileReaderFunc is an instance of a mock function object
// controlling the behavior of the method NewFileReader.
NewFileReaderFunc *GitserverClientNewFileReaderFunc
// RevListFunc is an instance of a mock function object controlling the
// behavior of the method RevList.
RevListFunc *GitserverClientRevListFunc
@ -59,8 +59,8 @@ func NewMockGitserverClient() *MockGitserverClient {
return
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (r0 []byte, r1 error) {
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (r0 io.ReadCloser, r1 error) {
return
},
},
@ -91,9 +91,9 @@ func NewStrictMockGitserverClient() *MockGitserverClient {
panic("unexpected invocation of MockGitserverClient.LogReverseEach")
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, types.RepoCommitPath) ([]byte, error) {
panic("unexpected invocation of MockGitserverClient.ReadFile")
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
panic("unexpected invocation of MockGitserverClient.NewFileReader")
},
},
RevListFunc: &GitserverClientRevListFunc{
@ -118,8 +118,8 @@ func NewMockGitserverClientFrom(i gitserver.GitserverClient) *MockGitserverClien
LogReverseEachFunc: &GitserverClientLogReverseEachFunc{
defaultHook: i.LogReverseEach,
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: i.ReadFile,
NewFileReaderFunc: &GitserverClientNewFileReaderFunc{
defaultHook: i.NewFileReader,
},
RevListFunc: &GitserverClientRevListFunc{
defaultHook: i.RevList,
@ -471,35 +471,36 @@ func (c GitserverClientLogReverseEachFuncCall) Results() []interface{} {
return []interface{}{c.Result0}
}
// GitserverClientReadFileFunc describes the behavior when the ReadFile
// method of the parent MockGitserverClient instance is invoked.
type GitserverClientReadFileFunc struct {
defaultHook func(context.Context, types.RepoCommitPath) ([]byte, error)
hooks []func(context.Context, types.RepoCommitPath) ([]byte, error)
history []GitserverClientReadFileFuncCall
// GitserverClientNewFileReaderFunc describes the behavior when the
// NewFileReader method of the parent MockGitserverClient instance is
// invoked.
type GitserverClientNewFileReaderFunc struct {
defaultHook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)
hooks []func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)
history []GitserverClientNewFileReaderFuncCall
mutex sync.Mutex
}
// ReadFile delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockGitserverClient) ReadFile(v0 context.Context, v1 types.RepoCommitPath) ([]byte, error) {
r0, r1 := m.ReadFileFunc.nextHook()(v0, v1)
m.ReadFileFunc.appendCall(GitserverClientReadFileFuncCall{v0, v1, r0, r1})
// NewFileReader delegates to the next hook function in the queue and stores
// the parameter and result values of this invocation.
func (m *MockGitserverClient) NewFileReader(v0 context.Context, v1 types.RepoCommitPath) (io.ReadCloser, error) {
r0, r1 := m.NewFileReaderFunc.nextHook()(v0, v1)
m.NewFileReaderFunc.appendCall(GitserverClientNewFileReaderFuncCall{v0, v1, r0, r1})
return r0, r1
}
// SetDefaultHook sets function that is called when the ReadFile method of
// the parent MockGitserverClient instance is invoked and the hook queue is
// empty.
func (f *GitserverClientReadFileFunc) SetDefaultHook(hook func(context.Context, types.RepoCommitPath) ([]byte, error)) {
// SetDefaultHook sets function that is called when the NewFileReader method
// of the parent MockGitserverClient instance is invoked and the hook queue
// is empty.
func (f *GitserverClientNewFileReaderFunc) SetDefaultHook(hook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// ReadFile method of the parent MockGitserverClient instance invokes the
// hook at the front of the queue and discards it. After the queue is empty,
// the default hook function is invoked for any future action.
func (f *GitserverClientReadFileFunc) PushHook(hook func(context.Context, types.RepoCommitPath) ([]byte, error)) {
// NewFileReader method of the parent MockGitserverClient instance invokes
// the hook at the front of the queue and discards it. After the queue is
// empty, the default hook function is invoked for any future action.
func (f *GitserverClientNewFileReaderFunc) PushHook(hook func(context.Context, types.RepoCommitPath) (io.ReadCloser, error)) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
@ -507,20 +508,20 @@ func (f *GitserverClientReadFileFunc) PushHook(hook func(context.Context, types.
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *GitserverClientReadFileFunc) SetDefaultReturn(r0 []byte, r1 error) {
f.SetDefaultHook(func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) SetDefaultReturn(r0 io.ReadCloser, r1 error) {
f.SetDefaultHook(func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
return r0, r1
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *GitserverClientReadFileFunc) PushReturn(r0 []byte, r1 error) {
f.PushHook(func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) PushReturn(r0 io.ReadCloser, r1 error) {
f.PushHook(func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
return r0, r1
})
}
func (f *GitserverClientReadFileFunc) nextHook() func(context.Context, types.RepoCommitPath) ([]byte, error) {
func (f *GitserverClientNewFileReaderFunc) nextHook() func(context.Context, types.RepoCommitPath) (io.ReadCloser, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
@ -533,26 +534,26 @@ func (f *GitserverClientReadFileFunc) nextHook() func(context.Context, types.Rep
return hook
}
func (f *GitserverClientReadFileFunc) appendCall(r0 GitserverClientReadFileFuncCall) {
func (f *GitserverClientNewFileReaderFunc) appendCall(r0 GitserverClientNewFileReaderFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of GitserverClientReadFileFuncCall objects
// describing the invocations of this function.
func (f *GitserverClientReadFileFunc) History() []GitserverClientReadFileFuncCall {
// History returns a sequence of GitserverClientNewFileReaderFuncCall
// objects describing the invocations of this function.
func (f *GitserverClientNewFileReaderFunc) History() []GitserverClientNewFileReaderFuncCall {
f.mutex.Lock()
history := make([]GitserverClientReadFileFuncCall, len(f.history))
history := make([]GitserverClientNewFileReaderFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// GitserverClientReadFileFuncCall is an object that describes an invocation
// of method ReadFile on an instance of MockGitserverClient.
type GitserverClientReadFileFuncCall struct {
// GitserverClientNewFileReaderFuncCall is an object that describes an
// invocation of method NewFileReader on an instance of MockGitserverClient.
type GitserverClientNewFileReaderFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
@ -561,7 +562,7 @@ type GitserverClientReadFileFuncCall struct {
Arg1 types.RepoCommitPath
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 []byte
Result0 io.ReadCloser
// Result1 is the value of the 2nd result returned from this method
// invocation.
Result1 error
@ -569,13 +570,13 @@ type GitserverClientReadFileFuncCall struct {
// Args returns an interface slice containing the arguments of this
// invocation.
func (c GitserverClientReadFileFuncCall) Args() []interface{} {
func (c GitserverClientNewFileReaderFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c GitserverClientReadFileFuncCall) Results() []interface{} {
func (c GitserverClientNewFileReaderFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}

1
cmd/symbols/shared/BUILD.bazel
View File

@ -39,6 +39,7 @@ go_library(
"//internal/search/result",
"//internal/service",
"//internal/trace",
"//internal/types",
"//lib/errors",
"@com_github_sourcegraph_go_ctags//:go-ctags",
"@com_github_sourcegraph_log//:log",

11
cmd/symbols/shared/main.go
View File

@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"fmt"
"io"
"net/http"
"os"
"strconv"
@ -29,6 +30,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/observation"
"github.com/sourcegraph/sourcegraph/internal/service"
"github.com/sourcegraph/sourcegraph/internal/trace"
internaltypes "github.com/sourcegraph/sourcegraph/internal/types"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
@ -85,7 +87,14 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic
routines = append(routines, newRoutines...)
// Create HTTP server
handler := api.NewHandler(searchFunc, gitserverClient.ReadFile, handleStatus, ctagsBinary)
handler := api.NewHandler(searchFunc, func(ctx context.Context, rcp internaltypes.RepoCommitPath) ([]byte, error) {
r, err := gitserverClient.NewFileReader(ctx, rcp)
if err != nil {
return nil, err
}
defer r.Close()
return io.ReadAll(r)
}, handleStatus, ctagsBinary)
handler = handlePanic(logger, handler)
handler = trace.HTTPMiddleware(logger, handler, conf.DefaultClient())

8
cmd/worker/internal/embeddings/repo/handler.go
View File

@ -2,6 +2,7 @@ package repo
import (
"context"
"io"
"github.com/sourcegraph/log"
@ -240,7 +241,12 @@ type revisionFetcher struct {
}
func (r *revisionFetcher) Read(ctx context.Context, fileName string) ([]byte, error) {
return r.gitserver.ReadFile(ctx, r.repo, r.revision, fileName)
fr, err := r.gitserver.NewFileReader(ctx, r.repo, r.revision, fileName)
if err != nil {
return nil, err
}
defer fr.Close()
return io.ReadAll(fr)
}
func (r *revisionFetcher) List(ctx context.Context) ([]embed.FileEntry, error) {

130
internal/batches/sources/mocks_test.go generated
View File

@ -10989,9 +10989,6 @@ type MockGitserverClient struct {
// ReadDirFunc is an instance of a mock function object controlling the
// behavior of the method ReadDir.
ReadDirFunc *GitserverClientReadDirFunc
// ReadFileFunc is an instance of a mock function object controlling the
// behavior of the method ReadFile.
ReadFileFunc *GitserverClientReadFileFunc
// RefDescriptionsFunc is an instance of a mock function object
// controlling the behavior of the method RefDescriptions.
RefDescriptionsFunc *GitserverClientRefDescriptionsFunc
@ -11260,11 +11257,6 @@ func NewMockGitserverClient() *MockGitserverClient {
return
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, api.RepoName, api.CommitID, string) (r0 []byte, r1 error) {
return
},
},
RefDescriptionsFunc: &GitserverClientRefDescriptionsFunc{
defaultHook: func(context.Context, api.RepoName, ...string) (r0 map[string][]gitdomain.RefDescription, r1 error) {
return
@ -11562,11 +11554,6 @@ func NewStrictMockGitserverClient() *MockGitserverClient {
panic("unexpected invocation of MockGitserverClient.ReadDir")
},
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
panic("unexpected invocation of MockGitserverClient.ReadFile")
},
},
RefDescriptionsFunc: &GitserverClientRefDescriptionsFunc{
defaultHook: func(context.Context, api.RepoName, ...string) (map[string][]gitdomain.RefDescription, error) {
panic("unexpected invocation of MockGitserverClient.RefDescriptions")
@ -11777,9 +11764,6 @@ func NewMockGitserverClientFrom(i gitserver.Client) *MockGitserverClient {
ReadDirFunc: &GitserverClientReadDirFunc{
defaultHook: i.ReadDir,
},
ReadFileFunc: &GitserverClientReadFileFunc{
defaultHook: i.ReadFile,
},
RefDescriptionsFunc: &GitserverClientRefDescriptionsFunc{
defaultHook: i.RefDescriptions,
},
@ -16780,120 +16764,6 @@ func (c GitserverClientReadDirFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// GitserverClientReadFileFunc describes the behavior when the ReadFile
// method of the parent MockGitserverClient instance is invoked.
type GitserverClientReadFileFunc struct {
defaultHook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)
hooks []func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)
history []GitserverClientReadFileFuncCall
mutex sync.Mutex
}
// ReadFile delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockGitserverClient) ReadFile(v0 context.Context, v1 api.RepoName, v2 api.CommitID, v3 string) ([]byte, error) {
r0, r1 := m.ReadFileFunc.nextHook()(v0, v1, v2, v3)
m.ReadFileFunc.appendCall(GitserverClientReadFileFuncCall{v0, v1, v2, v3, r0, r1})
return r0, r1
}
// SetDefaultHook sets function that is called when the ReadFile method of
// the parent MockGitserverClient instance is invoked and the hook queue is
// empty.
func (f *GitserverClientReadFileFunc) SetDefaultHook(hook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// ReadFile method of the parent MockGitserverClient instance invokes the
// hook at the front of the queue and discards it. After the queue is empty,
// the default hook function is invoked for any future action.
func (f *GitserverClientReadFileFunc) PushHook(hook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
}
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *GitserverClientReadFileFunc) SetDefaultReturn(r0 []byte, r1 error) {
f.SetDefaultHook(func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
return r0, r1
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *GitserverClientReadFileFunc) PushReturn(r0 []byte, r1 error) {
f.PushHook(func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
return r0, r1
})
}
func (f *GitserverClientReadFileFunc) nextHook() func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
if len(f.hooks) == 0 {
return f.defaultHook
}
hook := f.hooks[0]
f.hooks = f.hooks[1:]
return hook
}
func (f *GitserverClientReadFileFunc) appendCall(r0 GitserverClientReadFileFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of GitserverClientReadFileFuncCall objects
// describing the invocations of this function.
func (f *GitserverClientReadFileFunc) History() []GitserverClientReadFileFuncCall {
f.mutex.Lock()
history := make([]GitserverClientReadFileFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// GitserverClientReadFileFuncCall is an object that describes an invocation
// of method ReadFile on an instance of MockGitserverClient.
type GitserverClientReadFileFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
// Arg1 is the value of the 2nd argument passed to this method
// invocation.
Arg1 api.RepoName
// Arg2 is the value of the 3rd argument passed to this method
// invocation.
Arg2 api.CommitID
// Arg3 is the value of the 4th argument passed to this method
// invocation.
Arg3 string
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 []byte
// Result1 is the value of the 2nd result returned from this method
// invocation.
Result1 error
}
// Args returns an interface slice containing the arguments of this
// invocation.
func (c GitserverClientReadFileFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1, c.Arg2, c.Arg3}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c GitserverClientReadFileFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// GitserverClientRefDescriptionsFunc describes the behavior when the
// RefDescriptions method of the parent MockGitserverClient instance is
// invoked.

9
internal/codeintel/autoindexing/internal/jobselector/job_selector.go
View File

@ -2,6 +2,7 @@ package jobselector
import (
"context"
"io"
"os"
"github.com/sourcegraph/log"
@ -174,7 +175,7 @@ func (s *JobSelector) getIndexRecordsFromConfigurationInRepository(ctx context.C
return nil, false, err
}
content, err := s.gitserverClient.ReadFile(ctx, repo.Name, api.CommitID(commit), "sourcegraph.yaml")
r, err := s.gitserverClient.NewFileReader(ctx, repo.Name, api.CommitID(commit), "sourcegraph.yaml")
if err != nil {
if os.IsNotExist(err) {
return nil, false, nil
@ -183,6 +184,12 @@ func (s *JobSelector) getIndexRecordsFromConfigurationInRepository(ctx context.C
return nil, false, err
}
content, err := io.ReadAll(r)
r.Close()
if err != nil {
return nil, false, err
}
indexConfiguration, err := config.UnmarshalYAML(content)
if err != nil {
// We failed here, but do not try to fall back on another method as having

8
internal/codeintel/autoindexing/service_test.go
View File

@ -1,8 +1,10 @@
package autoindexing
import (
"bytes"
"context"
"fmt"
"io"
"os"
"sort"
"testing"
@ -259,7 +261,7 @@ func TestQueueIndexesInRepository(t *testing.T) {
gitserverClient.ResolveRevisionFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName, rev string, opts gitserver.ResolveRevisionOptions) (api.CommitID, error) {
return api.CommitID(fmt.Sprintf("c%s", repo)), nil
})
gitserverClient.ReadFileFunc.SetDefaultReturn(yamlIndexConfiguration, nil)
gitserverClient.NewFileReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader(yamlIndexConfiguration)), nil)
inferenceService := NewMockInferenceService()
service := newService(
@ -334,7 +336,7 @@ func TestQueueIndexesInferred(t *testing.T) {
gitserverClient.ResolveRevisionFunc.SetDefaultHook(func(ctx context.Context, repo api.RepoName, rev string, opts gitserver.ResolveRevisionOptions) (api.CommitID, error) {
return api.CommitID(fmt.Sprintf("c%s", repo)), nil
})
gitserverClient.ReadFileFunc.SetDefaultReturn(nil, os.ErrNotExist)
gitserverClient.NewFileReaderFunc.SetDefaultReturn(nil, os.ErrNotExist)
inferenceService := NewMockInferenceService()
inferenceService.InferIndexJobsFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, s1, s2 string) (*shared.InferenceResult, error) {
@ -407,7 +409,7 @@ func TestQueueIndexesForPackage(t *testing.T) {
}
return "c42", nil
})
gitserverClient.ReadFileFunc.SetDefaultReturn(nil, os.ErrNotExist)
gitserverClient.NewFileReaderFunc.SetDefaultReturn(nil, os.ErrNotExist)
inferenceService := NewMockInferenceService()
inferenceService.InferIndexJobsFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, s1, s2 string) (*shared.InferenceResult, error) {

8
internal/codeintel/codenav/service.go
View File

@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"io"
"strings"
"github.com/sourcegraph/log"
@ -983,7 +984,12 @@ func (s *Service) SnapshotForDocument(ctx context.Context, repositoryID int, com
return nil, err
}
file, err := s.gitserver.ReadFile(ctx, api.RepoName(dump.RepositoryName), api.CommitID(dump.Commit), path)
r, err := s.gitserver.NewFileReader(ctx, api.RepoName(dump.RepositoryName), api.CommitID(dump.Commit), path)
if err != nil {
return nil, err
}
file, err := io.ReadAll(r)
r.Close()
if err != nil {
return nil, err
}

4
internal/codeintel/codenav/service_snapshot_test.go
View File

@ -1,7 +1,9 @@
package codenav
import (
"bytes"
"context"
"io"
"testing"
"github.com/sourcegraph/scip/bindings/go/scip"
@ -28,7 +30,7 @@ func TestSnapshotForDocument(t *testing.T) {
mockUploadSvc.GetDumpsByIDsFunc.SetDefaultReturn([]shared.Dump{{}}, nil)
mockRepoStore.GetFunc.SetDefaultReturn(&types.Repo{}, nil)
mockGitserverClient.ReadFileFunc.SetDefaultReturn([]byte(sampleFile1), nil)
mockGitserverClient.NewFileReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte(sampleFile1))), nil)
mockLsifStore.SCIPDocumentFunc.SetDefaultReturn(&scip.Document{
RelativePath: "burger.go",
Occurrences: []*scip.Occurrence{{

8
internal/codeintel/shared/resolvers/gitresolvers/tree_entry.go
View File

@ -3,6 +3,7 @@ package gitresolvers
import (
"context"
"fmt"
"io"
stdpath "path"
"strings"
"time"
@ -54,7 +55,7 @@ func (r *treeEntryResolver) Content(ctx context.Context, args *resolvers.GitTree
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
content, err := r.gitserverClient.ReadFile(
fr, err := r.gitserverClient.NewFileReader(
ctx,
api.RepoName(r.commit.Repository().Name()), // repository name
api.CommitID(r.commit.OID()), // commit oid
@ -63,6 +64,11 @@ func (r *treeEntryResolver) Content(ctx context.Context, args *resolvers.GitTree
if err != nil {
return "", err
}
defer fr.Close()
content, err := io.ReadAll(fr)
if err != nil {
return "", err
}
return joinSelection(strings.Split(string(content), "\n"), args.StartLine, args.EndLine), nil
}

8
internal/compute/replace_command.go
View File

@ -3,6 +3,7 @@ package compute
import (
"context"
"fmt"
"io"
"github.com/sourcegraph/log"
@ -53,7 +54,12 @@ func replace(ctx context.Context, content []byte, matchPattern MatchPattern, rep
func (c *Replace) Run(ctx context.Context, gitserverClient gitserver.Client, r result.Match) (Result, error) {
switch m := r.(type) {
case *result.FileMatch:
content, err := gitserverClient.ReadFile(ctx, m.Repo.Name, m.CommitID, m.Path)
r, err := gitserverClient.NewFileReader(ctx, m.Repo.Name, m.CommitID, m.Path)
if err != nil {
return nil, err
}
defer r.Close()
content, err := io.ReadAll(r)
if err != nil {
return nil, err
}

12
internal/gitserver/client.go
View File

@ -358,6 +358,14 @@ type Client interface {
// NewFileReader returns an io.ReadCloser reading from the named file at commit.
// The caller should always close the reader after use.
//
// If you just need to check a file's existence, use Stat, not a file reader.
//
// If the file doesn't exist, the returned error will pass the os.IsNotExist()
// check.
//
// If the path points to a submodule, a reader for an empty file is returned
// (ie. io.EOF is returned immediately).
NewFileReader(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (io.ReadCloser, error)
// DiffSymbols performs a diff command which is expected to be parsed by our symbols package
@ -382,10 +390,6 @@ type Client interface {
// longer required.
Diff(ctx context.Context, opts DiffOptions) (*DiffFileIterator, error)
// ReadFile returns the full contents of the named file at commit.
// (If you just need to check a file's existence, use Stat, not ReadFile.)
ReadFile(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) ([]byte, error)
// BranchesContaining returns a map from branch names to branch tip hashes for
// each branch containing the given commit.
BranchesContaining(ctx context.Context, repo api.RepoName, commit api.CommitID) ([]string, error)

105
internal/gitserver/commands.go
View File

@ -1318,33 +1318,6 @@ func (c *clientImplementor) GetBehindAhead(ctx context.Context, repo api.RepoNam
return &gitdomain.BehindAhead{Behind: uint32(b), Ahead: uint32(a)}, nil
}
// ReadFile returns the full contents of the named file at commit.
// (If you just need to check a file's existence, use Stat, not ReadFile.)
func (c *clientImplementor) ReadFile(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (_ []byte, err error) {
ctx, _, endObservation := c.operations.readFile.With(ctx, &err, observation.Args{
MetricLabelValues: []string{c.scope},
Attrs: []attribute.KeyValue{
repo.Attr(),
commit.Attr(),
attribute.String("name", name),
},
})
defer endObservation(1, observation.Args{})
br, err := c.NewFileReader(ctx, repo, commit, name)
if err != nil {
return nil, err
}
defer br.Close()
r := io.Reader(br)
data, err := io.ReadAll(r)
if err != nil {
return nil, err
}
return data, nil
}
// NewFileReader returns an io.ReadCloser reading from the named file at commit.
// The caller should always close the reader after use
func (c *clientImplementor) NewFileReader(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (_ io.ReadCloser, err error) {
@ -1369,22 +1342,12 @@ func (c *clientImplementor) NewFileReader(ctx context.Context, repo api.RepoName
name = rel(name)
br, err := c.newBlobReader(ctx, repo, commit, name)
if err != nil {
return nil, errors.Wrapf(err, "getting blobReader for %q", name)
return nil, err
}
return br, nil
}
// blobReader, which should be created using newBlobReader, is a struct that allows
// us to get a ReadCloser to a specific named file at a specific commit
type blobReader struct {
c *clientImplementor
ctx context.Context
repo api.RepoName
commit api.CommitID
name string
cmd GitCommand
rc io.ReadCloser
}
var errIsSubmodule = errors.New("blob is a submodule")
func (c *clientImplementor) blobOID(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (string, error) {
// Note: when our git is new enough we can just use --object-only
@ -1403,54 +1366,46 @@ func (c *clientImplementor) blobOID(ctx context.Context, repo api.RepoName, comm
if len(fields) < 3 {
return "", errors.Newf("unexpected output while parsing blob OID: %q", string(out))
}
if string(fields[0]) == "160000" {
return "", errIsSubmodule
}
return string(fields[2]), nil
}
func (c *clientImplementor) newBlobReader(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (*blobReader, error) {
func (c *clientImplementor) newBlobReader(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (io.ReadCloser, error) {
if err := gitdomain.EnsureAbsoluteCommit(commit); err != nil {
return nil, err
}
var cmd GitCommand
if strings.Contains(name, "..") {
// We special case ".." in path to running a less efficient two
// commands. For other paths we can rely on the faster git show.
//
// git show will try and resolve revisions on anything containing
// "..". Depending on what branches/files exist, this can lead to:
//
// - error: object $SHA is a tree, not a commit
// - fatal: Invalid symmetric difference expression $SHA:$name
// - outputting a diff instead of the file
//
// The last point is a security issue for repositories with sub-repo
// permissions since the diff will not be filtered.
blobOID, err := c.blobOID(ctx, repo, commit, name)
if err != nil {
return nil, err
blobOID, err := c.blobOID(ctx, repo, commit, name)
if err != nil {
if err == errIsSubmodule {
return io.NopCloser(bytes.NewReader(nil)), nil
}
cmd = c.gitCommand(repo, "cat-file", "-p", blobOID)
} else {
// Otherwise we can rely on a single command git show sha:name.
cmd = c.gitCommand(repo, "show", string(commit)+":"+name)
return nil, err
}
cmd := c.gitCommand(repo, "cat-file", "-p", blobOID)
stdout, err := cmd.StdoutReader(ctx)
if err != nil {
return nil, err
}
return &blobReader{
c: c,
ctx: ctx,
repo: repo,
commit: commit,
name: name,
cmd: cmd,
rc: stdout,
name: name,
cmd: cmd,
rc: stdout,
}, nil
}
// blobReader, which should be created using newBlobReader, is a struct that allows
// us to get a ReadCloser to a specific named file at a specific commit
type blobReader struct {
name string
cmd GitCommand
rc io.ReadCloser
}
func (br *blobReader) Read(p []byte) (int, error) {
n, err := br.rc.Read(p)
if err != nil {
@ -1471,20 +1426,6 @@ func (br *blobReader) convertError(err error) error {
if err == io.EOF {
return err
}
if strings.Contains(err.Error(), "exists on disk, but not in") || strings.Contains(err.Error(), "does not exist") {
return &os.PathError{Op: "open", Path: br.name, Err: os.ErrNotExist}
}
if strings.Contains(err.Error(), "fatal: bad object ") {
// Could be a git submodule.
fi, err := br.c.Stat(br.ctx, br.repo, br.commit, br.name)
if err != nil {
return err
}
// Return EOF for a submodule for now which indicates zero content
if fi.Mode()&gitdomain.ModeSubmodule != 0 {
return io.EOF
}
}
return errors.WithMessage(err, fmt.Sprintf("git command %v failed (output: %q)", br.cmd.Args(), err))
}

38
internal/gitserver/commands_test.go
View File

@ -2421,41 +2421,6 @@ func TestRead(t *testing.T) {
}
}
t.Run(name+"-ReadFile", func(t *testing.T) {
client := NewTestClient(t).WithChecker(checker)
data, err := client.ReadFile(ctx, repo, commitID, test.file)
checkFn(t, err, data)
})
t.Run(name+"-ReadFile-with-sub-repo-permissions-no-op", func(t *testing.T) {
checker.EnabledFunc.SetDefaultHook(func() bool {
return true
})
checker.PermissionsFunc.SetDefaultHook(func(ctx context.Context, i int32, content authz.RepoContent) (authz.Perms, error) {
if content.Path == test.file {
return authz.Read, nil
}
return authz.None, nil
})
client := NewTestClient(t).WithChecker(checker)
data, err := client.ReadFile(ctx, repo, commitID, test.file)
checkFn(t, err, data)
})
t.Run(name+"-ReadFile-with-sub-repo-permissions-filters-file", func(t *testing.T) {
checker.EnabledFunc.SetDefaultHook(func() bool {
return true
})
checker.PermissionsFunc.SetDefaultHook(func(ctx context.Context, i int32, content authz.RepoContent) (authz.Perms, error) {
return authz.None, nil
})
client := NewTestClient(t).WithChecker(checker)
data, err := client.ReadFile(ctx, repo, commitID, test.file)
if err != os.ErrNotExist {
t.Errorf("unexpected error reading file: %s", err)
}
if string(data) != "" {
t.Errorf("unexpected data: %s", data)
}
})
t.Run(name+"-GetFileReader", func(t *testing.T) {
runNewFileReaderTest(ctx, t, repo, commitID, test.file, nil, checkFn)
})
@ -2503,7 +2468,8 @@ func runNewFileReaderTest(ctx context.Context, t *testing.T, repo api.RepoName,
t.Fatal(err)
}
}()
data, err := io.ReadAll(rc)
data, dataErr := io.ReadAll(rc)
require.NoError(t, dataErr)
checkFn(t, err, data)
}

6
internal/gitserver/git_command.go
View File

@ -8,7 +8,6 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"
"syscall"
"github.com/sourcegraph/log"
@ -114,11 +113,6 @@ func (l *LocalGitCommand) DividedOutput(ctx context.Context) ([]byte, []byte, er
}
l.exitStatus = exitStatus
// We want to treat actions on files that don't exist as an os.ErrNotExist
if err != nil && strings.Contains(stderrBuf.String(), "does not exist in") {
err = os.ErrNotExist
}
return stdoutBuf.Bytes(), bytes.TrimSpace(stderrBuf.Bytes()), err
}

129
internal/gitserver/mocks_temp.go
View File

@ -157,9 +157,6 @@ type MockClient struct {
// ReadDirFunc is an instance of a mock function object controlling the
// behavior of the method ReadDir.
ReadDirFunc *ClientReadDirFunc
// ReadFileFunc is an instance of a mock function object controlling the
// behavior of the method ReadFile.
ReadFileFunc *ClientReadFileFunc
// RefDescriptionsFunc is an instance of a mock function object
// controlling the behavior of the method RefDescriptions.
RefDescriptionsFunc *ClientRefDescriptionsFunc
@ -428,11 +425,6 @@ func NewMockClient() *MockClient {
return
},
},
ReadFileFunc: &ClientReadFileFunc{
defaultHook: func(context.Context, api.RepoName, api.CommitID, string) (r0 []byte, r1 error) {
return
},
},
RefDescriptionsFunc: &ClientRefDescriptionsFunc{
defaultHook: func(context.Context, api.RepoName, ...string) (r0 map[string][]gitdomain.RefDescription, r1 error) {
return
@ -730,11 +722,6 @@ func NewStrictMockClient() *MockClient {
panic("unexpected invocation of MockClient.ReadDir")
},
},
ReadFileFunc: &ClientReadFileFunc{
defaultHook: func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
panic("unexpected invocation of MockClient.ReadFile")
},
},
RefDescriptionsFunc: &ClientRefDescriptionsFunc{
defaultHook: func(context.Context, api.RepoName, ...string) (map[string][]gitdomain.RefDescription, error) {
panic("unexpected invocation of MockClient.RefDescriptions")
@ -944,9 +931,6 @@ func NewMockClientFrom(i Client) *MockClient {
ReadDirFunc: &ClientReadDirFunc{
defaultHook: i.ReadDir,
},
ReadFileFunc: &ClientReadFileFunc{
defaultHook: i.ReadFile,
},
RefDescriptionsFunc: &ClientRefDescriptionsFunc{
defaultHook: i.RefDescriptions,
},
@ -5868,119 +5852,6 @@ func (c ClientReadDirFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// ClientReadFileFunc describes the behavior when the ReadFile method of the
// parent MockClient instance is invoked.
type ClientReadFileFunc struct {
defaultHook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)
hooks []func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)
history []ClientReadFileFuncCall
mutex sync.Mutex
}
// ReadFile delegates to the next hook function in the queue and stores the
// parameter and result values of this invocation.
func (m *MockClient) ReadFile(v0 context.Context, v1 api.RepoName, v2 api.CommitID, v3 string) ([]byte, error) {
r0, r1 := m.ReadFileFunc.nextHook()(v0, v1, v2, v3)
m.ReadFileFunc.appendCall(ClientReadFileFuncCall{v0, v1, v2, v3, r0, r1})
return r0, r1
}
// SetDefaultHook sets function that is called when the ReadFile method of
// the parent MockClient instance is invoked and the hook queue is empty.
func (f *ClientReadFileFunc) SetDefaultHook(hook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)) {
f.defaultHook = hook
}
// PushHook adds a function to the end of hook queue. Each invocation of the
// ReadFile method of the parent MockClient instance invokes the hook at the
// front of the queue and discards it. After the queue is empty, the default
// hook function is invoked for any future action.
func (f *ClientReadFileFunc) PushHook(hook func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error)) {
f.mutex.Lock()
f.hooks = append(f.hooks, hook)
f.mutex.Unlock()
}
// SetDefaultReturn calls SetDefaultHook with a function that returns the
// given values.
func (f *ClientReadFileFunc) SetDefaultReturn(r0 []byte, r1 error) {
f.SetDefaultHook(func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
return r0, r1
})
}
// PushReturn calls PushHook with a function that returns the given values.
func (f *ClientReadFileFunc) PushReturn(r0 []byte, r1 error) {
f.PushHook(func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
return r0, r1
})
}
func (f *ClientReadFileFunc) nextHook() func(context.Context, api.RepoName, api.CommitID, string) ([]byte, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
if len(f.hooks) == 0 {
return f.defaultHook
}
hook := f.hooks[0]
f.hooks = f.hooks[1:]
return hook
}
func (f *ClientReadFileFunc) appendCall(r0 ClientReadFileFuncCall) {
f.mutex.Lock()
f.history = append(f.history, r0)
f.mutex.Unlock()
}
// History returns a sequence of ClientReadFileFuncCall objects describing
// the invocations of this function.
func (f *ClientReadFileFunc) History() []ClientReadFileFuncCall {
f.mutex.Lock()
history := make([]ClientReadFileFuncCall, len(f.history))
copy(history, f.history)
f.mutex.Unlock()
return history
}
// ClientReadFileFuncCall is an object that describes an invocation of
// method ReadFile on an instance of MockClient.
type ClientReadFileFuncCall struct {
// Arg0 is the value of the 1st argument passed to this method
// invocation.
Arg0 context.Context
// Arg1 is the value of the 2nd argument passed to this method
// invocation.
Arg1 api.RepoName
// Arg2 is the value of the 3rd argument passed to this method
// invocation.
Arg2 api.CommitID
// Arg3 is the value of the 4th argument passed to this method
// invocation.
Arg3 string
// Result0 is the value of the 1st result returned from this method
// invocation.
Result0 []byte
// Result1 is the value of the 2nd result returned from this method
// invocation.
Result1 error
}
// Args returns an interface slice containing the arguments of this
// invocation.
func (c ClientReadFileFuncCall) Args() []interface{} {
return []interface{}{c.Arg0, c.Arg1, c.Arg2, c.Arg3}
}
// Results returns an interface slice containing the results of this
// invocation.
func (c ClientReadFileFuncCall) Results() []interface{} {
return []interface{}{c.Result0, c.Result1}
}
// ClientRefDescriptionsFunc describes the behavior when the RefDescriptions
// method of the parent MockClient instance is invoked.
type ClientRefDescriptionsFunc struct {

2
internal/gitserver/observability.go
View File

@ -32,7 +32,6 @@ type operations struct {
mergeBase *observation.Operation
newFileReader *observation.Operation
readDir *observation.Operation
readFile *observation.Operation
resolveRevision *observation.Operation
revList *observation.Operation
search *observation.Operation
@ -138,7 +137,6 @@ func newOperations(observationCtx *observation.Context) *operations {
mergeBase: op("MergeBase"),
newFileReader: op("NewFileReader"),
readDir: op("ReadDir"),
readFile: op("ReadFile"),
resolveRevision: resolveRevisionOperation,
revList: op("RevList"),
search: op("Search"),

9
internal/own/background/analytics_test.go
View File

@ -1,15 +1,18 @@
package background
import (
"bytes"
"context"
"io"
"os"
"testing"
"time"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/database"
@ -34,7 +37,7 @@ func (f fakeGitServer) ResolveRevision(ctx context.Context, repo api.RepoName, s
return api.CommitID(""), nil
}
func (f fakeGitServer) ReadFile(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) ([]byte, error) {
func (f fakeGitServer) NewFileReader(ctx context.Context, repo api.RepoName, commit api.CommitID, name string) (io.ReadCloser, error) {
if f.fileContents == nil {
return nil, os.ErrNotExist
}
@ -42,7 +45,7 @@ func (f fakeGitServer) ReadFile(ctx context.Context, repo api.RepoName, commit a
if !ok {
return nil, os.ErrNotExist
}
return []byte(contents), nil
return io.NopCloser(bytes.NewReader([]byte(contents))), nil
}
func TestAnalyticsIndexerSuccess(t *testing.T) {

7
internal/own/search/filter_job_test.go
View File

@ -1,12 +1,15 @@
package search
import (
"bytes"
"context"
"io"
"io/fs"
"strings"
"testing"
"github.com/hexops/autogold/v2"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/database/dbmocks"
@ -537,12 +540,12 @@ func TestApplyCodeOwnershipFiltering(t *testing.T) {
ctx := context.Background()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, file string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, file string) (io.ReadCloser, error) {
content, ok := tt.args.repoContent[file]
if !ok {
return nil, fs.ErrNotExist
}
return []byte(content), nil
return io.NopCloser(bytes.NewReader([]byte(content))), nil
})
codeownersStore := dbmocks.NewMockCodeownersStore()

17
internal/own/search/select_job_test.go
View File

@ -1,13 +1,17 @@
package search
import (
"bytes"
"context"
"hash/fnv"
"io"
"io/fs"
"sort"
"testing"
"github.com/hexops/autogold/v2"
"github.com/stretchr/testify/assert"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/database/dbmocks"
@ -18,7 +22,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/search/result"
"github.com/sourcegraph/sourcegraph/internal/search/streaming"
"github.com/sourcegraph/sourcegraph/internal/types"
"github.com/stretchr/testify/assert"
)
func TestGetCodeOwnersFromMatches(t *testing.T) {
@ -39,9 +42,7 @@ func TestGetCodeOwnersFromMatches(t *testing.T) {
ctx := context.Background()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, file string) ([]byte, error) {
return nil, fs.ErrNotExist
})
gitserverClient.NewFileReaderFunc.SetDefaultReturn(nil, fs.ErrNotExist)
rules := NewRulesCache(gitserverClient, setupDB())
@ -63,9 +64,9 @@ func TestGetCodeOwnersFromMatches(t *testing.T) {
ctx := context.Background()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, file string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, s string) (io.ReadCloser, error) {
// return a codeowner path for no which doesn't match the path of the match below.
return []byte("NO.md @test\n"), nil
return io.NopCloser(bytes.NewReader([]byte("NO.md @test\n"))), nil
})
rules := NewRulesCache(gitserverClient, setupDB())
@ -87,10 +88,10 @@ func TestGetCodeOwnersFromMatches(t *testing.T) {
ctx := context.Background()
gitserverClient := gitserver.NewMockClient()
gitserverClient.ReadFileFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, _ api.CommitID, file string) ([]byte, error) {
gitserverClient.NewFileReaderFunc.SetDefaultHook(func(ctx context.Context, rn api.RepoName, ci api.CommitID, s string) (io.ReadCloser, error) {
// README is owned by a user and a team.
// code.go is owner by another user and an unknown entity.
return []byte("README.md @testUserHandle @testTeamHandle\ncode.go user@email.com @unknown"), nil
return io.NopCloser(bytes.NewReader([]byte("README.md @testUserHandle @testTeamHandle\ncode.go user@email.com @unknown"))), nil
})
mockUserStore := dbmocks.NewMockUserStore()
mockTeamStore := dbmocks.NewMockTeamStore()

25
internal/own/service.go
View File

@ -1,7 +1,6 @@
package own
import (
"bytes"
"context"
"os"
"strings"
@ -109,23 +108,27 @@ func (s *service) RulesetForRepo(ctx context.Context, repoName api.RepoName, rep
rs = codeowners.NewRuleset(codeowners.IngestedRulesetSource{ID: int32(ingestedCodeowners.RepoID)}, ingestedCodeowners.Proto)
} else {
for _, path := range codeownersLocations {
content, err := s.gitserverClient.ReadFile(
r, err := s.gitserverClient.NewFileReader(
ctx,
repoName,
commitID,
path,
)
if content != nil && err == nil {
pbfile, err := codeowners.Parse(bytes.NewReader(content))
if err != nil {
return nil, err
if err != nil {
if os.IsNotExist(err) {
continue
}
rs = codeowners.NewRuleset(codeowners.GitRulesetSource{Repo: repoID, Commit: commitID, Path: path}, pbfile)
break
} else if os.IsNotExist(err) {
continue
return nil, err
}
return nil, err
pbfile, err := codeowners.Parse(r)
r.Close()
if err != nil {
return nil, err
}
rs = codeowners.NewRuleset(codeowners.GitRulesetSource{Repo: repoID, Commit: commitID, Path: path}, pbfile)
break
}
}
if rs == nil {

13
internal/own/service_test.go
View File

@ -1,7 +1,9 @@
package own
import (
"bytes"
"context"
"io"
"os"
"sort"
"testing"
@ -12,6 +14,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/sourcegraph/log/logtest"
types2 "github.com/sourcegraph/sourcegraph/internal/types"
"github.com/sourcegraph/sourcegraph/internal/api"
@ -43,12 +46,12 @@ type repoPath struct {
// repoFiles is a fake git client mapping a file
type repoFiles map[repoPath]string
func (fs repoFiles) ReadFile(_ context.Context, repoName api.RepoName, commitID api.CommitID, file string) ([]byte, error) {
func (fs repoFiles) NewFileReader(_ context.Context, repoName api.RepoName, commitID api.CommitID, file string) (io.ReadCloser, error) {
content, ok := fs[repoPath{Repo: repoName, CommitID: commitID, Path: file}]
if !ok {
return nil, os.ErrNotExist
}
return []byte(content), nil
return io.NopCloser(bytes.NewReader([]byte(content))), nil
}
func TestOwnersServesFilesAtVariousLocations(t *testing.T) {
@ -70,7 +73,7 @@ func TestOwnersServesFilesAtVariousLocations(t *testing.T) {
} {
t.Run(name, func(t *testing.T) {
git := gitserver.NewMockClient()
git.ReadFileFunc.SetDefaultHook(repo.ReadFile)
git.NewFileReaderFunc.SetDefaultHook(repo.NewFileReader)
reposStore := dbmocks.NewMockRepoStore()
reposStore.GetFunc.SetDefaultReturn(&types2.Repo{ExternalRepo: api.ExternalRepoSpec{ServiceType: "github"}}, nil)
@ -103,7 +106,7 @@ func TestOwnersCannotFindFile(t *testing.T) {
{"repo", "SHA", "notCODEOWNERS"}: codeownersFile.Repr(),
}
git := gitserver.NewMockClient()
git.ReadFileFunc.SetDefaultHook(repo.ReadFile)
git.NewFileReaderFunc.SetDefaultHook(repo.NewFileReader)
codeownersStore := dbmocks.NewMockCodeownersStore()
codeownersStore.GetCodeownersForRepoFunc.SetDefaultReturn(nil, database.CodeownersFileNotFoundError{})
@ -147,7 +150,7 @@ func TestOwnersServesIngestedFile(t *testing.T) {
})
t.Run("file not found and codeowners file does not exist return nil", func(t *testing.T) {
git := gitserver.NewMockClient()
git.ReadFileFunc.SetDefaultReturn(nil, nil)
git.NewFileReaderFunc.SetDefaultReturn(nil, os.ErrNotExist)
codeownersStore := dbmocks.NewMockCodeownersStore()
codeownersStore.GetCodeownersForRepoFunc.SetDefaultReturn(nil, database.CodeownersFileNotFoundError{})