mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 17:51:57 +00:00
gitserver: Remove ResolveRevisions from client (#60484)
To migrate ResolveRevision to gRPC, I'm removing this endpoint in favor of individual calls for now. This will simplify the migration. If someone was requesting hundreds of revs here, this will actually give us better metrics about that, and we'll be able to fix it again. Test plan: Adjusted existing tests.
This commit is contained in:
Erik Seliger
GitHub
parent
8e250b408b
commit
2093bc8862
@ -62,7 +62,6 @@ go_test(
|
||||
"//internal/extsvc",
|
||||
"//internal/gitserver",
|
||||
"//internal/gitserver/gitdomain",
|
||||
"//internal/gitserver/protocol",
|
||||
"//internal/gitserver/v1:gitserver",
|
||||
"//internal/grpc",
|
||||
"//internal/grpc/defaults",
|
||||
|
||||
@ -23,7 +23,6 @@ import (
|
||||
"github.com/sourcegraph/sourcegraph/internal/extsvc"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
|
||||
proto "github.com/sourcegraph/sourcegraph/internal/gitserver/v1"
|
||||
internalgrpc "github.com/sourcegraph/sourcegraph/internal/grpc"
|
||||
"github.com/sourcegraph/sourcegraph/internal/grpc/defaults"
|
||||
@ -33,7 +32,7 @@ import (
|
||||
"github.com/sourcegraph/sourcegraph/internal/wrexec"
|
||||
)
|
||||
|
||||
func TestClient_ResolveRevisions(t *testing.T) {
|
||||
func TestClient_ResolveRevision(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
remote := createSimpleGitRepo(t, root)
|
||||
// These hashes should be stable since we set the timestamps
|
||||
@ -42,27 +41,27 @@ func TestClient_ResolveRevisions(t *testing.T) {
|
||||
hash2 := "c5151eceb40d5e625716589b745248e1a6c6228d"
|
||||
|
||||
tests := []struct {
|
||||
input []protocol.RevisionSpecifier
|
||||
want []string
|
||||
input string
|
||||
want api.CommitID
|
||||
err error
|
||||
}{{
|
||||
input: []protocol.RevisionSpecifier{{}},
|
||||
want: []string{hash2},
|
||||
input: "",
|
||||
want: api.CommitID(hash2),
|
||||
}, {
|
||||
input: []protocol.RevisionSpecifier{{RevSpec: "HEAD"}},
|
||||
want: []string{hash2},
|
||||
input: "HEAD",
|
||||
want: api.CommitID(hash2),
|
||||
}, {
|
||||
input: []protocol.RevisionSpecifier{{RevSpec: "HEAD~1"}},
|
||||
want: []string{hash1},
|
||||
input: "HEAD~1",
|
||||
want: api.CommitID(hash1),
|
||||
}, {
|
||||
input: []protocol.RevisionSpecifier{{RevSpec: "test-ref"}},
|
||||
want: []string{hash1},
|
||||
input: "test-ref",
|
||||
want: api.CommitID(hash1),
|
||||
}, {
|
||||
input: []protocol.RevisionSpecifier{{RevSpec: "test-nested-ref"}},
|
||||
want: []string{hash1},
|
||||
input: "test-nested-ref",
|
||||
want: api.CommitID(hash1),
|
||||
}, {
|
||||
input: []protocol.RevisionSpecifier{{RevSpec: "test-fake-ref"}},
|
||||
err: &gitdomain.RevisionNotFoundError{Repo: api.RepoName(remote), Spec: "test-fake-ref"},
|
||||
input: "test-fake-ref",
|
||||
err: &gitdomain.RevisionNotFoundError{Repo: api.RepoName(remote), Spec: "test-fake-ref^0"},
|
||||
}}
|
||||
|
||||
logger := logtest.Scoped(t)
|
||||
@ -107,7 +106,7 @@ func TestClient_ResolveRevisions(t *testing.T) {
|
||||
_, err := cli.RequestRepoUpdate(ctx, api.RepoName(remote), 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
got, err := cli.ResolveRevisions(ctx, api.RepoName(remote), test.input)
|
||||
got, err := cli.ResolveRevision(ctx, api.RepoName(remote), test.input, gitserver.ResolveRevisionOptions{NoEnsureRevision: true})
|
||||
if test.err != nil {
|
||||
require.Equal(t, test.err, err)
|
||||
return
|
||||
|
||||
130
internal/batches/sources/mocks_test.go
generated
130
internal/batches/sources/mocks_test.go
generated
@ -10986,9 +10986,6 @@ type MockGitserverClient struct {
|
||||
// ResolveRevisionFunc is an instance of a mock function object
|
||||
// controlling the behavior of the method ResolveRevision.
|
||||
ResolveRevisionFunc *GitserverClientResolveRevisionFunc
|
||||
// ResolveRevisionsFunc is an instance of a mock function object
|
||||
// controlling the behavior of the method ResolveRevisions.
|
||||
ResolveRevisionsFunc *GitserverClientResolveRevisionsFunc
|
||||
// RevListFunc is an instance of a mock function object controlling the
|
||||
// behavior of the method RevList.
|
||||
RevListFunc *GitserverClientRevListFunc
|
||||
@ -11231,11 +11228,6 @@ func NewMockGitserverClient() *MockGitserverClient {
|
||||
return
|
||||
},
|
||||
},
|
||||
ResolveRevisionsFunc: &GitserverClientResolveRevisionsFunc{
|
||||
defaultHook: func(context.Context, api.RepoName, []protocol.RevisionSpecifier) (r0 []string, r1 error) {
|
||||
return
|
||||
},
|
||||
},
|
||||
RevListFunc: &GitserverClientRevListFunc{
|
||||
defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) (r0 error) {
|
||||
return
|
||||
@ -11493,11 +11485,6 @@ func NewStrictMockGitserverClient() *MockGitserverClient {
|
||||
panic("unexpected invocation of MockGitserverClient.ResolveRevision")
|
||||
},
|
||||
},
|
||||
ResolveRevisionsFunc: &GitserverClientResolveRevisionsFunc{
|
||||
defaultHook: func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
panic("unexpected invocation of MockGitserverClient.ResolveRevisions")
|
||||
},
|
||||
},
|
||||
RevListFunc: &GitserverClientRevListFunc{
|
||||
defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) error {
|
||||
panic("unexpected invocation of MockGitserverClient.RevList")
|
||||
@ -11670,9 +11657,6 @@ func NewMockGitserverClientFrom(i gitserver.Client) *MockGitserverClient {
|
||||
ResolveRevisionFunc: &GitserverClientResolveRevisionFunc{
|
||||
defaultHook: i.ResolveRevision,
|
||||
},
|
||||
ResolveRevisionsFunc: &GitserverClientResolveRevisionsFunc{
|
||||
defaultHook: i.ResolveRevisions,
|
||||
},
|
||||
RevListFunc: &GitserverClientRevListFunc{
|
||||
defaultHook: i.RevList,
|
||||
},
|
||||
@ -16563,120 +16547,6 @@ func (c GitserverClientResolveRevisionFuncCall) Results() []interface{} {
|
||||
return []interface{}{c.Result0, c.Result1}
|
||||
}
|
||||
|
||||
// GitserverClientResolveRevisionsFunc describes the behavior when the
|
||||
// ResolveRevisions method of the parent MockGitserverClient instance is
|
||||
// invoked.
|
||||
type GitserverClientResolveRevisionsFunc struct {
|
||||
defaultHook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)
|
||||
hooks []func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)
|
||||
history []GitserverClientResolveRevisionsFuncCall
|
||||
mutex sync.Mutex
|
||||
}
|
||||
|
||||
// ResolveRevisions delegates to the next hook function in the queue and
|
||||
// stores the parameter and result values of this invocation.
|
||||
func (m *MockGitserverClient) ResolveRevisions(v0 context.Context, v1 api.RepoName, v2 []protocol.RevisionSpecifier) ([]string, error) {
|
||||
r0, r1 := m.ResolveRevisionsFunc.nextHook()(v0, v1, v2)
|
||||
m.ResolveRevisionsFunc.appendCall(GitserverClientResolveRevisionsFuncCall{v0, v1, v2, r0, r1})
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
// SetDefaultHook sets function that is called when the ResolveRevisions
|
||||
// method of the parent MockGitserverClient instance is invoked and the hook
|
||||
// queue is empty.
|
||||
func (f *GitserverClientResolveRevisionsFunc) SetDefaultHook(hook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)) {
|
||||
f.defaultHook = hook
|
||||
}
|
||||
|
||||
// PushHook adds a function to the end of hook queue. Each invocation of the
|
||||
// ResolveRevisions 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 *GitserverClientResolveRevisionsFunc) PushHook(hook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, 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 *GitserverClientResolveRevisionsFunc) SetDefaultReturn(r0 []string, r1 error) {
|
||||
f.SetDefaultHook(func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
return r0, r1
|
||||
})
|
||||
}
|
||||
|
||||
// PushReturn calls PushHook with a function that returns the given values.
|
||||
func (f *GitserverClientResolveRevisionsFunc) PushReturn(r0 []string, r1 error) {
|
||||
f.PushHook(func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
return r0, r1
|
||||
})
|
||||
}
|
||||
|
||||
func (f *GitserverClientResolveRevisionsFunc) nextHook() func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, 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 *GitserverClientResolveRevisionsFunc) appendCall(r0 GitserverClientResolveRevisionsFuncCall) {
|
||||
f.mutex.Lock()
|
||||
f.history = append(f.history, r0)
|
||||
f.mutex.Unlock()
|
||||
}
|
||||
|
||||
// History returns a sequence of GitserverClientResolveRevisionsFuncCall
|
||||
// objects describing the invocations of this function.
|
||||
func (f *GitserverClientResolveRevisionsFunc) History() []GitserverClientResolveRevisionsFuncCall {
|
||||
f.mutex.Lock()
|
||||
history := make([]GitserverClientResolveRevisionsFuncCall, len(f.history))
|
||||
copy(history, f.history)
|
||||
f.mutex.Unlock()
|
||||
|
||||
return history
|
||||
}
|
||||
|
||||
// GitserverClientResolveRevisionsFuncCall is an object that describes an
|
||||
// invocation of method ResolveRevisions on an instance of
|
||||
// MockGitserverClient.
|
||||
type GitserverClientResolveRevisionsFuncCall 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 []protocol.RevisionSpecifier
|
||||
// Result0 is the value of the 1st result returned from this method
|
||||
// invocation.
|
||||
Result0 []string
|
||||
// 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 GitserverClientResolveRevisionsFuncCall) Args() []interface{} {
|
||||
return []interface{}{c.Arg0, c.Arg1, c.Arg2}
|
||||
}
|
||||
|
||||
// Results returns an interface slice containing the results of this
|
||||
// invocation.
|
||||
func (c GitserverClientResolveRevisionsFuncCall) Results() []interface{} {
|
||||
return []interface{}{c.Result0, c.Result1}
|
||||
}
|
||||
|
||||
// GitserverClientRevListFunc describes the behavior when the RevList method
|
||||
// of the parent MockGitserverClient instance is invoked.
|
||||
type GitserverClientRevListFunc struct {
|
||||
|
||||
@ -107,15 +107,20 @@ func Snapshot(ctx context.Context, logger log.Logger, db database.DB, query stri
|
||||
)
|
||||
|
||||
hook := func(ctx context.Context, db database.DB, gs commit.GitserverClient, args *gitprotocol.SearchRequest, repoID api.RepoID, _ commit.DoSearchFunc) error {
|
||||
// Resolve the requested revisions into a static set of commit hashes
|
||||
commitHashes, err := gs.ResolveRevisions(ctx, args.Repo, args.Revisions)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, rev := range args.Revisions {
|
||||
// Fail early for context cancelation.
|
||||
if err := ctx.Err(); err != nil {
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
mu.Lock()
|
||||
resolvedRevisions[repoID] = commitHashes
|
||||
mu.Unlock()
|
||||
res, err := gs.ResolveRevision(ctx, args.Repo, rev, gitserver.ResolveRevisionOptions{NoEnsureRevision: true})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
mu.Lock()
|
||||
resolvedRevisions[repoID] = append(resolvedRevisions[repoID], string(res))
|
||||
mu.Unlock()
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
@ -174,9 +179,18 @@ func hookWithID(
|
||||
cm := db.CodeMonitors()
|
||||
|
||||
// Resolve the requested revisions into a static set of commit hashes
|
||||
commitHashes, err := gs.ResolveRevisions(ctx, args.Repo, args.Revisions)
|
||||
if err != nil {
|
||||
return err
|
||||
commitHashes := make([]string, 0, len(args.Revisions))
|
||||
for _, rev := range args.Revisions {
|
||||
// Fail early for context cancelation.
|
||||
if err := ctx.Err(); err != nil {
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
res, err := gs.ResolveRevision(ctx, args.Repo, rev, gitserver.ResolveRevisionOptions{NoEnsureRevision: true})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
commitHashes = append(commitHashes, string(res))
|
||||
}
|
||||
|
||||
// Look up the previously searched set of commit hashes
|
||||
@ -190,12 +204,10 @@ func hookWithID(
|
||||
}
|
||||
|
||||
// Merge requested hashes and excluded hashes
|
||||
newRevs := make([]gitprotocol.RevisionSpecifier, 0, len(commitHashes)+len(lastSearched))
|
||||
for _, hash := range commitHashes {
|
||||
newRevs = append(newRevs, gitprotocol.RevisionSpecifier{RevSpec: hash})
|
||||
}
|
||||
newRevs := make([]string, 0, len(commitHashes)+len(lastSearched))
|
||||
newRevs = append(newRevs, commitHashes...)
|
||||
for _, exclude := range lastSearched {
|
||||
newRevs = append(newRevs, gitprotocol.RevisionSpecifier{RevSpec: "^" + exclude})
|
||||
newRevs = append(newRevs, "^"+exclude)
|
||||
}
|
||||
|
||||
// Update args with the new set of revisions
|
||||
|
||||
@ -138,44 +138,35 @@ func TestCodeMonitorHook(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
gs := gitserver.NewMockClient()
|
||||
gs.ResolveRevisionsFunc.PushReturn([]string{"hash1", "hash2"}, nil)
|
||||
gs.ResolveRevisionsFunc.PushReturn([]string{"hash3", "hash4"}, nil)
|
||||
gs.ResolveRevisionsFunc.PushReturn([]string{"hash5", "hash6"}, nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash1", nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash2", nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash3", nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash4", nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash5", nil)
|
||||
gs.ResolveRevisionFunc.PushReturn("hash6", nil)
|
||||
|
||||
// The first time, doSearch should receive only the resolved hashes
|
||||
doSearch := func(args *gitprotocol.SearchRequest) error {
|
||||
require.Equal(t, args.Revisions, []gitprotocol.RevisionSpecifier{{
|
||||
RevSpec: "hash1",
|
||||
}, {
|
||||
RevSpec: "hash2",
|
||||
}})
|
||||
require.Equal(t, args.Revisions, []string{"hash1", "hash2"})
|
||||
return nil
|
||||
}
|
||||
err := hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{}, doSearch)
|
||||
err := hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{Revisions: []string{"rev1", "rev2"}}, doSearch)
|
||||
require.NoError(t, err)
|
||||
|
||||
// The next time, doSearch should receive the new resolved hashes plus the
|
||||
// hashes from last time, but excluded
|
||||
doSearch = func(args *gitprotocol.SearchRequest) error {
|
||||
require.Equal(t, args.Revisions, []gitprotocol.RevisionSpecifier{{
|
||||
RevSpec: "hash3",
|
||||
}, {
|
||||
RevSpec: "hash4",
|
||||
}, {
|
||||
RevSpec: "^hash1",
|
||||
}, {
|
||||
RevSpec: "^hash2",
|
||||
}})
|
||||
require.Equal(t, args.Revisions, []string{"hash3", "hash4", "^hash1", "^hash2"})
|
||||
return nil
|
||||
}
|
||||
err = hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{}, doSearch)
|
||||
err = hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{Revisions: []string{"rev1", "rev2"}}, doSearch)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Run("deadline exceeded is propagated", func(t *testing.T) {
|
||||
doSearch = func(args *gitprotocol.SearchRequest) error {
|
||||
return context.DeadlineExceeded
|
||||
}
|
||||
err := hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{}, doSearch)
|
||||
err := hookWithID(ctx, db, gs, fixtures.Monitor.ID, fixtures.Repo.ID, &gitprotocol.SearchRequest{Revisions: []string{"rev1", "rev2"}}, doSearch)
|
||||
require.ErrorContains(t, err, "some commits may be skipped")
|
||||
})
|
||||
}
|
||||
|
||||
@ -27,7 +27,6 @@ import (
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
|
||||
proto "github.com/sourcegraph/sourcegraph/internal/gitserver/v1"
|
||||
"github.com/sourcegraph/sourcegraph/internal/grpc/streamio"
|
||||
"github.com/sourcegraph/sourcegraph/internal/lazyregexp"
|
||||
"github.com/sourcegraph/sourcegraph/internal/observation"
|
||||
"github.com/sourcegraph/sourcegraph/internal/perforce"
|
||||
"github.com/sourcegraph/sourcegraph/lib/errors"
|
||||
@ -361,9 +360,6 @@ type Client interface {
|
||||
// * Other unexpected errors.
|
||||
ResolveRevision(ctx context.Context, repo api.RepoName, spec string, opt ResolveRevisionOptions) (api.CommitID, error)
|
||||
|
||||
// ResolveRevisions expands a set of RevisionSpecifiers into an equivalent set of commit hashes.
|
||||
ResolveRevisions(_ context.Context, repo api.RepoName, _ []protocol.RevisionSpecifier) ([]string, error)
|
||||
|
||||
// RequestRepoUpdate is the new protocol endpoint for synchronous requests
|
||||
// with more detailed responses. Do not use this if you are not repo-updater.
|
||||
//
|
||||
@ -1263,78 +1259,6 @@ func (c *clientImplementor) GetObject(ctx context.Context, repo api.RepoName, ob
|
||||
return &res.Object, nil
|
||||
}
|
||||
|
||||
var ambiguousArgPattern = lazyregexp.New(`ambiguous argument '([^']+)'`)
|
||||
|
||||
func (c *clientImplementor) ResolveRevisions(ctx context.Context, repo api.RepoName, revs []protocol.RevisionSpecifier) (_ []string, err error) {
|
||||
ctx, _, endObservation := c.operations.resolveRevisions.With(ctx, &err, observation.Args{
|
||||
MetricLabelValues: []string{c.scope},
|
||||
Attrs: []attribute.KeyValue{
|
||||
repo.Attr(),
|
||||
attribute.Int("revs", len(revs)),
|
||||
},
|
||||
})
|
||||
defer endObservation(1, observation.Args{})
|
||||
|
||||
args := append([]string{"rev-parse"}, revsToGitArgs(revs)...)
|
||||
|
||||
cmd := c.gitCommand(repo, args...)
|
||||
stdout, stderr, err := cmd.DividedOutput(ctx)
|
||||
if err != nil {
|
||||
if gitdomain.IsRepoNotExist(err) {
|
||||
return nil, err
|
||||
}
|
||||
if match := ambiguousArgPattern.FindSubmatch(stderr); match != nil {
|
||||
return nil, &gitdomain.RevisionNotFoundError{Repo: repo, Spec: string(match[1])}
|
||||
}
|
||||
return nil, errors.WithMessage(err, fmt.Sprintf("git command %v failed (stderr: %q)", cmd.Args(), stderr))
|
||||
}
|
||||
|
||||
return strings.Fields(string(stdout)), nil
|
||||
}
|
||||
|
||||
func revsToGitArgs(revSpecs []protocol.RevisionSpecifier) []string {
|
||||
args := make([]string, 0, len(revSpecs))
|
||||
for _, r := range revSpecs {
|
||||
if r.RevSpec != "" {
|
||||
args = append(args, r.RevSpec)
|
||||
} else {
|
||||
args = append(args, "HEAD")
|
||||
}
|
||||
}
|
||||
|
||||
// If revSpecs is empty, git treats it as equivalent to HEAD
|
||||
if len(revSpecs) == 0 {
|
||||
args = append(args, "HEAD")
|
||||
}
|
||||
return args
|
||||
}
|
||||
|
||||
// readResponseBody will attempt to read the body of the HTTP response and return it as a
|
||||
// string. However, in the unlikely scenario that it fails to read the body, it will encode and
|
||||
// return the error message as a string.
|
||||
//
|
||||
// This allows us to use this function directly without yet another if err != nil check. As a
|
||||
// result, this function should **only** be used when we're attempting to return the body's content
|
||||
// as part of an error. In such scenarios we don't need to return the potential error from reading
|
||||
// the body, but can get away with returning that error as a string itself.
|
||||
//
|
||||
// This is an unusual pattern of not returning an error. Be careful of replicating this in other
|
||||
// parts of the code.
|
||||
func readResponseBody(body io.Reader) string {
|
||||
content, err := io.ReadAll(body)
|
||||
if err != nil {
|
||||
return fmt.Sprintf("failed to read response body, error: %v", err)
|
||||
}
|
||||
|
||||
// strings.TrimSpace is needed to remove trailing \n characters that is added by the
|
||||
// server. We use http.Error in the server which in turn uses fmt.Fprintln to format
|
||||
// the error message. And in translation that newline gets escaped into a \n
|
||||
// character. For what the error message would look in the UI without
|
||||
// strings.TrimSpace, see attached screenshots in this pull request:
|
||||
// https://github.com/sourcegraph/sourcegraph/pull/39358.
|
||||
return strings.TrimSpace(string(content))
|
||||
}
|
||||
|
||||
func stringsToByteSlices(in []string) [][]byte {
|
||||
res := make([][]byte, len(in))
|
||||
for i, s := range in {
|
||||
|
||||
@ -34,7 +34,6 @@ import (
|
||||
"github.com/sourcegraph/sourcegraph/internal/byteutils"
|
||||
"github.com/sourcegraph/sourcegraph/internal/fileutil"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
|
||||
proto "github.com/sourcegraph/sourcegraph/internal/gitserver/v1"
|
||||
"github.com/sourcegraph/sourcegraph/internal/grpc/streamio"
|
||||
"github.com/sourcegraph/sourcegraph/internal/lazyregexp"
|
||||
@ -436,12 +435,6 @@ func (c *clientImplementor) DiffSymbols(ctx context.Context, repo api.RepoName,
|
||||
})
|
||||
defer endObservation(1, observation.Args{})
|
||||
|
||||
// Ensure commits exist to provide a better error message
|
||||
_, err = c.ResolveRevisions(ctx, repo, []protocol.RevisionSpecifier{{RevSpec: string(commitA)}, {RevSpec: string(commitB)}})
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "failed to lookup revisions for git diff on %s between %s and %s", repo, commitA, commitB)
|
||||
}
|
||||
|
||||
command := c.gitCommand(repo, "diff", "-z", "--name-status", "--no-renames", string(commitA), string(commitB))
|
||||
out, err := command.Output(ctx)
|
||||
if err != nil {
|
||||
|
||||
@ -1,11 +1,8 @@
|
||||
package gitserver
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
)
|
||||
|
||||
func BenchmarkAddrForKey(b *testing.B) {
|
||||
@ -22,14 +19,3 @@ func BenchmarkAddrForKey(b *testing.B) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_readResponseBody(t *testing.T) {
|
||||
// The \n in the end is important to test that readResponseBody correctly removes it from the returned string.
|
||||
reader := bytes.NewReader([]byte("A test string that is more than 40 bytes long. Lorem ipsum whatever whatever\n"))
|
||||
|
||||
expected := "A test string that is more than 40 bytes long. Lorem ipsum whatever whatever"
|
||||
got := readResponseBody(reader)
|
||||
if diff := cmp.Diff([]byte(expected), []byte(got)); diff != "" {
|
||||
t.Fatalf("Mismatch (-want +got):\n%s", diff)
|
||||
}
|
||||
}
|
||||
|
||||
@ -154,9 +154,6 @@ type MockClient struct {
|
||||
// ResolveRevisionFunc is an instance of a mock function object
|
||||
// controlling the behavior of the method ResolveRevision.
|
||||
ResolveRevisionFunc *ClientResolveRevisionFunc
|
||||
// ResolveRevisionsFunc is an instance of a mock function object
|
||||
// controlling the behavior of the method ResolveRevisions.
|
||||
ResolveRevisionsFunc *ClientResolveRevisionsFunc
|
||||
// RevListFunc is an instance of a mock function object controlling the
|
||||
// behavior of the method RevList.
|
||||
RevListFunc *ClientRevListFunc
|
||||
@ -399,11 +396,6 @@ func NewMockClient() *MockClient {
|
||||
return
|
||||
},
|
||||
},
|
||||
ResolveRevisionsFunc: &ClientResolveRevisionsFunc{
|
||||
defaultHook: func(context.Context, api.RepoName, []protocol.RevisionSpecifier) (r0 []string, r1 error) {
|
||||
return
|
||||
},
|
||||
},
|
||||
RevListFunc: &ClientRevListFunc{
|
||||
defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) (r0 error) {
|
||||
return
|
||||
@ -661,11 +653,6 @@ func NewStrictMockClient() *MockClient {
|
||||
panic("unexpected invocation of MockClient.ResolveRevision")
|
||||
},
|
||||
},
|
||||
ResolveRevisionsFunc: &ClientResolveRevisionsFunc{
|
||||
defaultHook: func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
panic("unexpected invocation of MockClient.ResolveRevisions")
|
||||
},
|
||||
},
|
||||
RevListFunc: &ClientRevListFunc{
|
||||
defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) error {
|
||||
panic("unexpected invocation of MockClient.RevList")
|
||||
@ -837,9 +824,6 @@ func NewMockClientFrom(i Client) *MockClient {
|
||||
ResolveRevisionFunc: &ClientResolveRevisionFunc{
|
||||
defaultHook: i.ResolveRevision,
|
||||
},
|
||||
ResolveRevisionsFunc: &ClientResolveRevisionsFunc{
|
||||
defaultHook: i.ResolveRevisions,
|
||||
},
|
||||
RevListFunc: &ClientRevListFunc{
|
||||
defaultHook: i.RevList,
|
||||
},
|
||||
@ -5646,117 +5630,6 @@ func (c ClientResolveRevisionFuncCall) Results() []interface{} {
|
||||
return []interface{}{c.Result0, c.Result1}
|
||||
}
|
||||
|
||||
// ClientResolveRevisionsFunc describes the behavior when the
|
||||
// ResolveRevisions method of the parent MockClient instance is invoked.
|
||||
type ClientResolveRevisionsFunc struct {
|
||||
defaultHook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)
|
||||
hooks []func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)
|
||||
history []ClientResolveRevisionsFuncCall
|
||||
mutex sync.Mutex
|
||||
}
|
||||
|
||||
// ResolveRevisions delegates to the next hook function in the queue and
|
||||
// stores the parameter and result values of this invocation.
|
||||
func (m *MockClient) ResolveRevisions(v0 context.Context, v1 api.RepoName, v2 []protocol.RevisionSpecifier) ([]string, error) {
|
||||
r0, r1 := m.ResolveRevisionsFunc.nextHook()(v0, v1, v2)
|
||||
m.ResolveRevisionsFunc.appendCall(ClientResolveRevisionsFuncCall{v0, v1, v2, r0, r1})
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
// SetDefaultHook sets function that is called when the ResolveRevisions
|
||||
// method of the parent MockClient instance is invoked and the hook queue is
|
||||
// empty.
|
||||
func (f *ClientResolveRevisionsFunc) SetDefaultHook(hook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error)) {
|
||||
f.defaultHook = hook
|
||||
}
|
||||
|
||||
// PushHook adds a function to the end of hook queue. Each invocation of the
|
||||
// ResolveRevisions 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 *ClientResolveRevisionsFunc) PushHook(hook func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, 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 *ClientResolveRevisionsFunc) SetDefaultReturn(r0 []string, r1 error) {
|
||||
f.SetDefaultHook(func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
return r0, r1
|
||||
})
|
||||
}
|
||||
|
||||
// PushReturn calls PushHook with a function that returns the given values.
|
||||
func (f *ClientResolveRevisionsFunc) PushReturn(r0 []string, r1 error) {
|
||||
f.PushHook(func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, error) {
|
||||
return r0, r1
|
||||
})
|
||||
}
|
||||
|
||||
func (f *ClientResolveRevisionsFunc) nextHook() func(context.Context, api.RepoName, []protocol.RevisionSpecifier) ([]string, 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 *ClientResolveRevisionsFunc) appendCall(r0 ClientResolveRevisionsFuncCall) {
|
||||
f.mutex.Lock()
|
||||
f.history = append(f.history, r0)
|
||||
f.mutex.Unlock()
|
||||
}
|
||||
|
||||
// History returns a sequence of ClientResolveRevisionsFuncCall objects
|
||||
// describing the invocations of this function.
|
||||
func (f *ClientResolveRevisionsFunc) History() []ClientResolveRevisionsFuncCall {
|
||||
f.mutex.Lock()
|
||||
history := make([]ClientResolveRevisionsFuncCall, len(f.history))
|
||||
copy(history, f.history)
|
||||
f.mutex.Unlock()
|
||||
|
||||
return history
|
||||
}
|
||||
|
||||
// ClientResolveRevisionsFuncCall is an object that describes an invocation
|
||||
// of method ResolveRevisions on an instance of MockClient.
|
||||
type ClientResolveRevisionsFuncCall 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 []protocol.RevisionSpecifier
|
||||
// Result0 is the value of the 1st result returned from this method
|
||||
// invocation.
|
||||
Result0 []string
|
||||
// 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 ClientResolveRevisionsFuncCall) Args() []interface{} {
|
||||
return []interface{}{c.Arg0, c.Arg1, c.Arg2}
|
||||
}
|
||||
|
||||
// Results returns an interface slice containing the results of this
|
||||
// invocation.
|
||||
func (c ClientResolveRevisionsFuncCall) Results() []interface{} {
|
||||
return []interface{}{c.Result0, c.Result1}
|
||||
}
|
||||
|
||||
// ClientRevListFunc describes the behavior when the RevList method of the
|
||||
// parent MockClient instance is invoked.
|
||||
type ClientRevListFunc struct {
|
||||
|
||||
@ -50,7 +50,6 @@ type operations struct {
|
||||
perforceGetChangelist *observation.Operation
|
||||
createCommitFromPatch *observation.Operation
|
||||
getObject *observation.Operation
|
||||
resolveRevisions *observation.Operation
|
||||
commitGraph *observation.Operation
|
||||
refDescriptions *observation.Operation
|
||||
branchesContaining *observation.Operation
|
||||
@ -148,7 +147,6 @@ func newOperations(observationCtx *observation.Context) *operations {
|
||||
perforceGetChangelist: op("PerforceGetChangelist"),
|
||||
createCommitFromPatch: op("CreateCommitFromPatch"),
|
||||
getObject: op("GetObject"),
|
||||
resolveRevisions: op("ResolveRevisions"),
|
||||
commitGraph: op("CommitGraph"),
|
||||
refDescriptions: op("RefDescriptions"),
|
||||
branchesContaining: op("BranchesContaining"),
|
||||
|
||||
@ -16,7 +16,7 @@ import (
|
||||
|
||||
type SearchRequest struct {
|
||||
Repo api.RepoName
|
||||
Revisions []RevisionSpecifier
|
||||
Revisions []string
|
||||
Query Node
|
||||
IncludeDiff bool
|
||||
Limit int
|
||||
@ -26,7 +26,7 @@ type SearchRequest struct {
|
||||
func (r *SearchRequest) ToProto() *proto.SearchRequest {
|
||||
revs := make([]*proto.RevisionSpecifier, 0, len(r.Revisions))
|
||||
for _, rev := range r.Revisions {
|
||||
revs = append(revs, rev.ToProto())
|
||||
revs = append(revs, &proto.RevisionSpecifier{RevSpec: rev})
|
||||
}
|
||||
return &proto.SearchRequest{
|
||||
Repo: string(r.Repo),
|
||||
@ -44,9 +44,9 @@ func SearchRequestFromProto(p *proto.SearchRequest) (*SearchRequest, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
revisions := make([]RevisionSpecifier, 0, len(p.GetRevisions()))
|
||||
revisions := make([]string, 0, len(p.GetRevisions()))
|
||||
for _, rev := range p.GetRevisions() {
|
||||
revisions = append(revisions, RevisionSpecifierFromProto(rev))
|
||||
revisions = append(revisions, rev.GetRevSpec())
|
||||
}
|
||||
|
||||
return &SearchRequest{
|
||||
@ -59,24 +59,6 @@ func SearchRequestFromProto(p *proto.SearchRequest) (*SearchRequest, error) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
type RevisionSpecifier struct {
|
||||
// RevSpec is a revision range specifier suitable for passing to git. See
|
||||
// the manpage gitrevisions(7).
|
||||
RevSpec string
|
||||
}
|
||||
|
||||
func (r *RevisionSpecifier) ToProto() *proto.RevisionSpecifier {
|
||||
return &proto.RevisionSpecifier{
|
||||
RevSpec: r.RevSpec,
|
||||
}
|
||||
}
|
||||
|
||||
func RevisionSpecifierFromProto(p *proto.RevisionSpecifier) RevisionSpecifier {
|
||||
return RevisionSpecifier{
|
||||
RevSpec: p.GetRevSpec(),
|
||||
}
|
||||
}
|
||||
|
||||
type SearchEventMatches []CommitMatch
|
||||
|
||||
type SearchEventDone struct {
|
||||
|
||||
@ -13,7 +13,7 @@ import (
|
||||
func TestSearchRequestProtoRoundtrip(t *testing.T) {
|
||||
req := &SearchRequest{
|
||||
Repo: "test1",
|
||||
Revisions: []RevisionSpecifier{{RevSpec: "ABC"}},
|
||||
Revisions: []string{"ABC"},
|
||||
Query: &Operator{
|
||||
Kind: And,
|
||||
Operands: []Node{
|
||||
|
||||
@ -85,7 +85,7 @@ type CommitSearcher struct {
|
||||
Logger log.Logger
|
||||
RepoDir string
|
||||
Query MatchTree
|
||||
Revisions []protocol.RevisionSpecifier
|
||||
Revisions []string
|
||||
IncludeDiff bool
|
||||
IncludeModifiedFiles bool
|
||||
RepoName api.RepoName
|
||||
@ -143,6 +143,18 @@ func (cs *CommitSearcher) gitArgs() []string {
|
||||
return args
|
||||
}
|
||||
|
||||
func revsToGitArgs(revs []string) []string {
|
||||
revArgs := make([]string, 0, len(revs))
|
||||
for _, rev := range revs {
|
||||
if rev != "" {
|
||||
revArgs = append(revArgs, rev)
|
||||
} else {
|
||||
revArgs = append(revArgs, "HEAD")
|
||||
}
|
||||
}
|
||||
return revArgs
|
||||
}
|
||||
|
||||
func (cs *CommitSearcher) feedBatches(ctx context.Context, jobs chan job, resultChans chan chan *protocol.CommitMatch) (err error) {
|
||||
cmd := exec.CommandContext(ctx, "git", cs.gitArgs()...)
|
||||
cmd.Dir = cs.RepoDir
|
||||
@ -263,18 +275,6 @@ func (cs *CommitSearcher) runJobs(ctx context.Context, jobs chan job) error {
|
||||
return errs
|
||||
}
|
||||
|
||||
func revsToGitArgs(revs []protocol.RevisionSpecifier) []string {
|
||||
revArgs := make([]string, 0, len(revs))
|
||||
for _, rev := range revs {
|
||||
if rev.RevSpec != "" {
|
||||
revArgs = append(revArgs, rev.RevSpec)
|
||||
} else {
|
||||
revArgs = append(revArgs, "HEAD")
|
||||
}
|
||||
}
|
||||
return revArgs
|
||||
}
|
||||
|
||||
// RawCommit is a shallow parse of the output of git log
|
||||
type RawCommit struct {
|
||||
Hash []byte
|
||||
|
||||
@ -756,19 +756,17 @@ func (authorNameGenerator) Generate(rand *rand.Rand, size int) reflect.Value {
|
||||
func Test_revsToGitArgs(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
revSpecs []protocol.RevisionSpecifier
|
||||
revSpecs []string
|
||||
expected []string
|
||||
}{
|
||||
{
|
||||
name: "explicit HEAD",
|
||||
revSpecs: []protocol.RevisionSpecifier{{
|
||||
RevSpec: "HEAD",
|
||||
}},
|
||||
name: "explicit HEAD",
|
||||
revSpecs: []string{"HEAD"},
|
||||
expected: []string{"HEAD"},
|
||||
},
|
||||
{
|
||||
name: "implicit HEAD",
|
||||
revSpecs: []protocol.RevisionSpecifier{{}},
|
||||
revSpecs: []string{""},
|
||||
expected: []string{"HEAD"},
|
||||
},
|
||||
}
|
||||
|
||||
@ -10,6 +10,7 @@ go_library(
|
||||
"//internal/api",
|
||||
"//internal/database",
|
||||
"//internal/errcode",
|
||||
"//internal/gitserver",
|
||||
"//internal/gitserver/gitdomain",
|
||||
"//internal/gitserver/protocol",
|
||||
"//internal/search",
|
||||
|
||||
@ -12,6 +12,7 @@ import (
|
||||
"github.com/sourcegraph/sourcegraph/internal/api"
|
||||
"github.com/sourcegraph/sourcegraph/internal/database"
|
||||
"github.com/sourcegraph/sourcegraph/internal/errcode"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
|
||||
"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
|
||||
gitprotocol "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
|
||||
@ -42,7 +43,7 @@ type CodeMonitorHook func(context.Context, database.DB, GitserverClient, *gitpro
|
||||
|
||||
type GitserverClient interface {
|
||||
Search(_ context.Context, _ *protocol.SearchRequest, onMatches func([]protocol.CommitMatch)) (limitHit bool, _ error)
|
||||
ResolveRevisions(context.Context, api.RepoName, []gitprotocol.RevisionSpecifier) ([]string, error)
|
||||
ResolveRevision(context.Context, api.RepoName, string, gitserver.ResolveRevisionOptions) (api.CommitID, error)
|
||||
}
|
||||
|
||||
func (j *SearchJob) Run(ctx context.Context, clients job.RuntimeClients, stream streaming.Sender) (alert *search.Alert, err error) {
|
||||
@ -61,7 +62,7 @@ func (j *SearchJob) Run(ctx context.Context, clients job.RuntimeClients, stream
|
||||
|
||||
args := &protocol.SearchRequest{
|
||||
Repo: repoRev.Repo.Name,
|
||||
Revisions: searchRevsToGitserverRevs(repoRev.Revs),
|
||||
Revisions: repoRev.Revs,
|
||||
Query: j.Query,
|
||||
IncludeDiff: j.Diff,
|
||||
Limit: j.Limit,
|
||||
@ -248,16 +249,6 @@ func QueryToGitQuery(b query.Basic, diff bool) gitprotocol.Node {
|
||||
return gitprotocol.Reduce(gitprotocol.NewAnd(res...))
|
||||
}
|
||||
|
||||
func searchRevsToGitserverRevs(in []string) []gitprotocol.RevisionSpecifier {
|
||||
out := make([]gitprotocol.RevisionSpecifier, 0, len(in))
|
||||
for _, rev := range in {
|
||||
out = append(out, gitprotocol.RevisionSpecifier{
|
||||
RevSpec: rev,
|
||||
})
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func queryPatternToPredicate(node query.Node, caseSensitive, diff bool) gitprotocol.Node {
|
||||
switch v := node.(type) {
|
||||
case query.Operator:
|
||||
|
||||
Loading…
Reference in New Issue
Block a user