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

Search: return parsed repo filters from queries (#46136)

Browse Source 
Currently the query classes `query.Q` and `query.Parameters` return repo
filters as a string. Callers needs to know that these filter strings can
contain revisions too (using the `repo@rev` syntax). So downstream code
constantly re-parse the filters, or sometimes forgets, which has caused several
bugs.

This PR updates `query.Q` and `query.Parameters` to parse the repo filters
up-front and return them as structs. This cleans up calling code and helps
prevents future mistakes.

Relates to #45632
This commit is contained in:
Julie Tibshirani 2023-01-06 14:06:04 -08:00 committed by GitHub
parent 7ff8676cd6
commit 977feecc9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 388 additions and 267 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cmd/frontend/graphqlbackend/search_test.go
View File

@ -290,7 +290,12 @@ func TestExactlyOneRepo(t *testing.T) {
}
for _, c := range cases {
t.Run("exactly one repo", func(t *testing.T) {
if got := searchrepos.ExactlyOneRepo(c.repoFilters); got != c.want {
parsedFilters := make([]query.ParsedRepoFilter, len(c.repoFilters))
for i, repoFilter := range c.repoFilters {
parsedFilters[i] = query.ParseRepositoryRevisions(repoFilter)
}
if got := searchrepos.ExactlyOneRepo(parsedFilters); got != c.want {
t.Errorf("got %t, want %t", got, c.want)
}
})

7
enterprise/internal/insights/resolvers/insight_series_resolver.go
View File

@ -11,7 +11,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/gqlutil"
"github.com/sourcegraph/sourcegraph/internal/metrics"
"github.com/sourcegraph/sourcegraph/internal/search"
"github.com/inconshreveable/log15"
"github.com/prometheus/client_golang/prometheus"
@ -69,12 +68,10 @@ func unwrapSearchContexts(ctx context.Context, loader SearchContextLoader, rawCo
}
inc, exc := plan.ToQ().Repositories()
for _, repoFilter := range inc {
repo, repoRevs := search.ParseRepositoryRevisions(repoFilter)
if len(repoRevs) > 0 {
if len(repoFilter.Revs) > 0 {
return nil, nil, errors.Errorf("search context filters cannot include repo revisions: %s", rawContext)
}
include = append(include, repo)
include = append(include, repoFilter.Repo)
}
exclude = append(exclude, exc...)
}

17
internal/search/job/jobutil/job.go
View File

@ -6,6 +6,8 @@ import (
"github.com/grafana/regexp"
zoektquery "github.com/sourcegraph/zoekt/query"
"github.com/sourcegraph/sourcegraph/internal/authz"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/search"
@ -24,7 +26,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/search/zoekt"
"github.com/sourcegraph/sourcegraph/lib/errors"
"github.com/sourcegraph/sourcegraph/schema"
zoektquery "github.com/sourcegraph/zoekt/query"
)
// NewPlanJob converts a query.Plan into its job tree representation.
@ -402,13 +403,14 @@ func NewFlatJob(searchInputs *search.Inputs, f query.Flat) (job.Job, error) {
return opts, true
}
opts.RepoFilters = append(make([]string, 0, len(opts.RepoFilters)), opts.RepoFilters...)
opts.RepoFilters = append(make([]query.ParsedRepoFilter, 0, len(opts.RepoFilters)), opts.RepoFilters...)
opts.CaseSensitiveRepoFilters = f.IsCaseSensitive()
patternPrefix := strings.SplitN(pattern, "@", 2)
if len(patternPrefix) == 1 {
// No "@" in pattern? We're good.
opts.RepoFilters = append(opts.RepoFilters, pattern)
repoFilter := query.ParseRepositoryRevisions(pattern)
opts.RepoFilters = append(opts.RepoFilters, repoFilter)
return opts, true
}
@ -426,7 +428,8 @@ func NewFlatJob(searchInputs *search.Inputs, f query.Flat) (job.Job, error) {
// a search. But fixing the order of concerns for repo code is not something @rvantonder is doing today.
return search.RepoOptions{}, false
}
opts.RepoFilters = append(opts.RepoFilters, patternPrefix[0])
repoFilter := query.ParseRepositoryRevisions(patternPrefix[0])
opts.RepoFilters = append(opts.RepoFilters, repoFilter)
return opts, true
}
@ -447,14 +450,14 @@ func NewFlatJob(searchInputs *search.Inputs, f query.Flat) (job.Job, error) {
repoNamePatterns := make([]*regexp.Regexp, 0, len(repoOptions.RepoFilters))
for _, repoFilter := range repoOptions.RepoFilters {
repoFilterPrefix, _ := search.ParseRepositoryRevisions(repoFilter)
// Because we pass the result of f.ToBasic().PatternString() to addPatternAsRepoFilter()
// above, regexp meta characters in literal patterns are already escaped before the
// pattern is added to repoOptions, so no need to check that before compiling to regex
if repoOptions.CaseSensitiveRepoFilters {
repoNamePatterns = append(repoNamePatterns, regexp.MustCompile(repoFilterPrefix))
repoNamePatterns = append(repoNamePatterns, regexp.MustCompile(repoFilter.Repo))
} else {
repoNamePatterns = append(repoNamePatterns, regexp.MustCompile(`(?i)`+repoFilterPrefix))
repoNamePatterns = append(repoNamePatterns, regexp.MustCompile(`(?i)`+repoFilter.Repo))
}
}

70
internal/search/job/jobutil/job_test.go
View File

@ -75,7 +75,7 @@ func TestNewPlanJob(t *testing.T) {
(SEARCHERTEXTSEARCH
(indexed . false))))
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)(repoOpts.searchContextSpec . @userA)
(repoOpts.repoFilters . [foo])(repoOpts.searchContextSpec . @userA)
(repoNamePatterns . [(?i)foo])))
(REPOSCOMPUTEEXCLUDED
(repoOpts.searchContextSpec . @userA))
@ -104,7 +104,7 @@ func TestNewPlanJob(t *testing.T) {
(type . text)
(repoOpts.searchContextSpec . global))
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)(repoOpts.searchContextSpec . global)
(repoOpts.repoFilters . [foo])(repoOpts.searchContextSpec . global)
(repoNamePatterns . [(?i)foo])))
(REPOSCOMPUTEEXCLUDED
(repoOpts.searchContextSpec . global))
@ -131,7 +131,7 @@ func TestNewPlanJob(t *testing.T) {
(type . text)
)
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)
(repoOpts.repoFilters . [foo])
(repoNamePatterns . [(?i)foo])))
(REPOSCOMPUTEEXCLUDED
)
@ -154,21 +154,21 @@ func TestNewPlanJob(t *testing.T) {
(SEQUENTIAL
(ensureUnique . false)
(REPOPAGER
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph)
(repoOpts.repoFilters . [sourcegraph/sourcegraph])
(PARTIALREPOS
(ZOEKTREPOSUBSETTEXTSEARCH
(query . substr:"foo")
(type . text))))
(REPOPAGER
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph)
(repoOpts.repoFilters . [sourcegraph/sourcegraph])
(PARTIALREPOS
(SEARCHERTEXTSEARCH
(indexed . false))))
(REPOSEARCH
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph)(repoOpts.repoFilters.1 . foo)
(repoOpts.repoFilters . [sourcegraph/sourcegraph foo])
(repoNamePatterns . [(?i)sourcegraph/sourcegraph (?i)foo])))
(REPOSCOMPUTEEXCLUDED
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph))
(repoOpts.repoFilters . [sourcegraph/sourcegraph]))
(PARALLEL
NoopJob
NoopJob))))))`),
@ -194,7 +194,7 @@ func TestNewPlanJob(t *testing.T) {
(type . text)
)
(REPOSEARCH
(repoOpts.repoFilters.0 . (?:ok).*?(?:ok))
(repoOpts.repoFilters . [(?:ok).*?(?:ok)])
(repoNamePatterns . [(?i)(?:ok).*?(?:ok)])))
(REPOSCOMPUTEEXCLUDED
)
@ -221,7 +221,7 @@ func TestNewPlanJob(t *testing.T) {
(type . text)
)
(REPOSEARCH
(repoOpts.repoFilters.0 . ok )
(repoOpts.repoFilters . [ok ])
(repoNamePatterns . [(?i)ok ])))
(REPOSCOMPUTEEXCLUDED
)
@ -265,9 +265,9 @@ func TestNewPlanJob(t *testing.T) {
(limit . 500)
(PARALLEL
(REPOSCOMPUTEEXCLUDED
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph@*refs/heads/*))
(repoOpts.repoFilters . [sourcegraph/sourcegraph@*refs/heads/*]))
(REPOSEARCH
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph@*refs/heads/*)
(repoOpts.repoFilters . [sourcegraph/sourcegraph@*refs/heads/*])
(repoNamePatterns . [(?i)sourcegraph/sourcegraph]))))))))`),
}, {
query: `repo:sourcegraph/sourcegraph@*refs/heads/*`,
@ -286,9 +286,9 @@ func TestNewPlanJob(t *testing.T) {
(limit . 500)
(PARALLEL
(REPOSCOMPUTEEXCLUDED
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph@*refs/heads/*))
(repoOpts.repoFilters . [sourcegraph/sourcegraph@*refs/heads/*]))
(REPOSEARCH
(repoOpts.repoFilters.0 . sourcegraph/sourcegraph@*refs/heads/*)
(repoOpts.repoFilters . [sourcegraph/sourcegraph@*refs/heads/*])
(repoNamePatterns . [(?i)sourcegraph/sourcegraph]))))))))`),
}, {
query: `foo @bar`,
@ -425,35 +425,35 @@ func TestNewPlanJob(t *testing.T) {
(SEQUENTIAL
(ensureUnique . false)
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(ZOEKTREPOSUBSETTEXTSEARCH
(query . substr:"test")
(type . text))))
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(SEARCHERTEXTSEARCH
(indexed . false))))
(REPOSEARCH
(repoOpts.repoFilters.0 . test)(repoOpts.repoFilters.1 . test)
(repoOpts.repoFilters . [test test])
(repoNamePatterns . [(?i)test (?i)test])))
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(ZOEKTSYMBOLSEARCH
(query . sym:substr:"test"))))
(COMMITSEARCH
(query . *protocol.MessageMatches(test))
(repoOpts.repoFilters.0 . test)(repoOpts.onlyCloned . true)
(repoOpts.repoFilters . [test])(repoOpts.onlyCloned . true)
(diff . false)
(limit . 500))
(REPOSCOMPUTEEXCLUDED
(repoOpts.repoFilters.0 . test))
(repoOpts.repoFilters . [test]))
(PARALLEL
NoopJob
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(SEARCHERSYMBOLSEARCH
(patternInfo.pattern . test)(patternInfo.isRegexp . true)(patternInfo.fileMatchLimit . 500)(patternInfo.patternMatchesPath . true)
@ -505,35 +505,35 @@ func TestNewPlanJob(t *testing.T) {
(SEQUENTIAL
(ensureUnique . false)
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(ZOEKTREPOSUBSETTEXTSEARCH
(query . substr:"test")
(type . text))))
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(SEARCHERTEXTSEARCH
(indexed . false))))
(REPOSEARCH
(repoOpts.repoFilters.0 . test)(repoOpts.repoFilters.1 . test)
(repoOpts.repoFilters . [test test])
(repoNamePatterns . [(?i)test (?i)test])))
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(ZOEKTSYMBOLSEARCH
(query . sym:substr:"test"))))
(COMMITSEARCH
(query . *protocol.MessageMatches(test))
(repoOpts.repoFilters.0 . test)(repoOpts.onlyCloned . true)
(repoOpts.repoFilters . [test])(repoOpts.onlyCloned . true)
(diff . false)
(limit . 500))
(REPOSCOMPUTEEXCLUDED
(repoOpts.repoFilters.0 . test))
(repoOpts.repoFilters . [test]))
(PARALLEL
NoopJob
(REPOPAGER
(repoOpts.repoFilters.0 . test)
(repoOpts.repoFilters . [test])
(PARTIALREPOS
(SEARCHERSYMBOLSEARCH
(patternInfo.pattern . test)(patternInfo.isRegexp . true)(patternInfo.fileMatchLimit . 500)(patternInfo.patternMatchesPath . true)
@ -601,7 +601,7 @@ func TestNewPlanJob(t *testing.T) {
(REPOSCOMPUTEEXCLUDED
)
(REPOSEARCH
(repoOpts.repoFilters.0 . a)
(repoOpts.repoFilters . [a])
(repoNamePatterns . [(?i)a])))))
(TIMEOUT
(timeout . 20s)
@ -749,7 +749,7 @@ func TestNewPlanJob(t *testing.T) {
(SEARCHERTEXTSEARCH
(indexed . false))))
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)(repoOpts.hasKVPs[0].key . tag)
(repoOpts.repoFilters . [foo])(repoOpts.hasKVPs[0].key . tag)
(repoNamePatterns . [(?i)foo])))
(REPOSCOMPUTEEXCLUDED
(repoOpts.hasKVPs[0].key . tag))
@ -817,13 +817,13 @@ func TestToEvaluateJob(t *testing.T) {
autogold.Want("root limit for streaming search", `
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)
(repoOpts.repoFilters . [foo])
(repoNamePatterns . [(?i)foo]))
`).Equal(t, test("foo", search.Streaming))
autogold.Want("root limit for batch search", `
(REPOSEARCH
(repoOpts.repoFilters.0 . foo)
(repoOpts.repoFilters . [foo])
(repoNamePatterns . [(?i)foo]))
`).Equal(t, test("foo", search.Batch))
}
@ -1373,16 +1373,16 @@ func TestZoektQueryPatternsAsRegexps(t *testing.T) {
func makeRepositoryRevisions(repos ...string) []*search.RepositoryRevisions {
r := make([]*search.RepositoryRevisions, len(repos))
for i, repospec := range repos {
repoName, revSpecs := search.ParseRepositoryRevisions(repospec)
revs := make([]string, 0, len(revSpecs))
for _, revSpec := range revSpecs {
repoRevs := query.ParseRepositoryRevisions(repospec)
revs := make([]string, 0, len(repoRevs.Revs))
for _, revSpec := range repoRevs.Revs {
revs = append(revs, revSpec.RevSpec)
}
if len(revs) == 0 {
// treat empty list as HEAD
revs = []string{""}
}
r[i] = &search.RepositoryRevisions{Repo: mkRepos(repoName)[0], Revs: revs}
r[i] = &search.RepositoryRevisions{Repo: mkRepos(repoRevs.Repo)[0], Revs: revs}
}
return r
}

120
internal/search/query/repo_revs.go Normal file
View File

@ -0,0 +1,120 @@
package query
import (
"strings"
)
// RevisionSpecifier represents either a revspec or a ref glob. At most one
// field is set. The default branch is represented by all fields being empty.
type RevisionSpecifier struct {
// RevSpec is a revision range specifier suitable for passing to git. See
// the manpage gitrevisions(7).
RevSpec string
// RefGlob is a reference glob to pass to git. See the documentation for
// "--glob" in git-log.
RefGlob string
// ExcludeRefGlob is a glob for references to exclude. See the
// documentation for "--exclude" in git-log.
ExcludeRefGlob string
}
func (r1 RevisionSpecifier) String() string {
if r1.ExcludeRefGlob != "" {
return "*!" + r1.ExcludeRefGlob
}
if r1.RefGlob != "" {
return "*" + r1.RefGlob
}
return r1.RevSpec
}
// Less compares two revspecOrRefGlob entities, suitable for use
// with sort.Slice()
//
// possibly-undesired: this results in treating an entity with
// no revspec, but a refGlob, as "earlier" than any revspec.
func (r1 RevisionSpecifier) Less(r2 RevisionSpecifier) bool {
if r1.RevSpec != r2.RevSpec {
return r1.RevSpec < r2.RevSpec
}
if r1.RefGlob != r2.RefGlob {
return r1.RefGlob < r2.RefGlob
}
return r1.ExcludeRefGlob < r2.ExcludeRefGlob
}
func (r1 RevisionSpecifier) HasRefGlob() bool {
return r1.RefGlob != "" || r1.ExcludeRefGlob != ""
}
type ParsedRepoFilter struct {
Repo string
Revs []RevisionSpecifier
}
func (p ParsedRepoFilter) String() string {
if len(p.Revs) == 0 {
return p.Repo
}
revSpecs := make([]string, len(p.Revs))
for i, r := range p.Revs {
revSpecs[i] = r.String()
}
return p.Repo + "@" + strings.Join(revSpecs, ":")
}
// ParseRepositoryRevisions parses strings that refer to a repository and 0
// or more revspecs. The format is:
//
// repo@revs
//
// where repo is a repository regex and revs is a ':'-separated list of revspecs
// and/or ref globs. A ref glob is a revspec prefixed with '*' (which is not a
// valid revspec or ref itself; see `man git-check-ref-format`). The '@' and revs
// may be omitted to refer to the default branch.
//
// For example:
//
// - 'foo' refers to the 'foo' repo at the default branch
// - 'foo@bar' refers to the 'foo' repo and the 'bar' revspec.
// - 'foo@bar:baz:qux' refers to the 'foo' repo and 3 revspecs: 'bar', 'baz',
// and 'qux'.
// - 'foo@*bar' refers to the 'foo' repo and all refs matching the glob 'bar/*',
// because git interprets the ref glob 'bar' as being 'bar/*' (see `man git-log`
// section on the --glob flag)
func ParseRepositoryRevisions(repoAndOptionalRev string) ParsedRepoFilter {
i := strings.Index(repoAndOptionalRev, "@")
if i == -1 {
// return an empty slice to indicate that there's no revisions; callers
// have to distinguish between "none specified" and "default" to handle
// cases where two repo specs both match the same repository, and only one
// specifies a revspec, which normally implies "master" but in that case
// really means "didn't specify"
return ParsedRepoFilter{Repo: repoAndOptionalRev, Revs: []RevisionSpecifier{}}
}
repo := repoAndOptionalRev[:i]
var revs []RevisionSpecifier
for _, part := range strings.Split(repoAndOptionalRev[i+1:], ":") {
if part == "" {
continue
}
revs = append(revs, parseRev(part))
}
if len(revs) == 0 {
revs = []RevisionSpecifier{{RevSpec: ""}} // default branch
}
return ParsedRepoFilter{Repo: repo, Revs: revs}
}
func parseRev(spec string) RevisionSpecifier {
if strings.HasPrefix(spec, "*!") {
return RevisionSpecifier{ExcludeRefGlob: spec[2:]}
} else if strings.HasPrefix(spec, "*") {
return RevisionSpecifier{RefGlob: spec[1:]}
}
return RevisionSpecifier{RevSpec: spec}
}

40
internal/search/query/repo_revs_test.go Normal file
View File

@ -0,0 +1,40 @@
package query
import (
"testing"
"github.com/google/go-cmp/cmp"
)
func TestParseRepositoryRevisions(t *testing.T) {
tests := map[string]ParsedRepoFilter{
"repo": {Repo: "repo", Revs: []RevisionSpecifier{}},
"repo@": {Repo: "repo", Revs: []RevisionSpecifier{{RevSpec: ""}}},
"repo@rev": {Repo: "repo", Revs: []RevisionSpecifier{{RevSpec: "rev"}}},
"repo@rev1:rev2": {Repo: "repo", Revs: []RevisionSpecifier{{RevSpec: "rev1"}, {RevSpec: "rev2"}}},
"repo@:rev1:": {Repo: "repo", Revs: []RevisionSpecifier{{RevSpec: "rev1"}}},
"repo@*glob": {Repo: "repo", Revs: []RevisionSpecifier{{RefGlob: "glob"}}},
"repo@rev1:*glob1:^rev2": {
Repo: "repo",
Revs: []RevisionSpecifier{{RevSpec: "rev1"}, {RefGlob: "glob1"}, {RevSpec: "^rev2"}},
},
"repo@rev1:*glob1:*!glob2:rev2:*glob3": {
Repo: "repo",
Revs: []RevisionSpecifier{
{RevSpec: "rev1"},
{RefGlob: "glob1"},
{ExcludeRefGlob: "glob2"},
{RevSpec: "rev2"},
{RefGlob: "glob3"},
},
},
}
for input, want := range tests {
t.Run(input, func(t *testing.T) {
repoRevs := ParseRepositoryRevisions(input)
if diff := cmp.Diff(want, repoRevs); diff != "" {
t.Fatalf("(-want +got):\n%s", diff)
}
})
}
}

10
internal/search/query/types.go
View File

@ -138,7 +138,7 @@ func (q Q) IsCaseSensitive() bool {
return q.BoolValue("case")
}
func (q Q) Repositories() (repos []string, negatedRepos []string) {
func (q Q) Repositories() (repos []ParsedRepoFilter, negatedRepos []string) {
VisitField(q, FieldRepo, func(value string, negated bool, a Annotation) {
if a.Labels.IsSet(IsPredicate) {
return
@ -147,7 +147,8 @@ func (q Q) Repositories() (repos []string, negatedRepos []string) {
if negated {
negatedRepos = append(negatedRepos, value)
} else {
repos = append(repos, value)
repoFilter := ParseRepositoryRevisions(value)
repos = append(repos, repoFilter)
}
})
return repos, negatedRepos
@ -555,7 +556,7 @@ func (p Parameters) Archived() *YesNoOnly {
return p.yesNoOnlyValue(FieldArchived)
}
func (p Parameters) Repositories() (repos []string, negatedRepos []string) {
func (p Parameters) Repositories() (repos []ParsedRepoFilter, negatedRepos []string) {
VisitField(toNodes(p), FieldRepo, func(value string, negated bool, a Annotation) {
if a.Labels.IsSet(IsPredicate) {
return
@ -564,7 +565,8 @@ func (p Parameters) Repositories() (repos []string, negatedRepos []string) {
if negated {
negatedRepos = append(negatedRepos, value)
} else {
repos = append(repos, value)
repoFilter := ParseRepositoryRevisions(value)
repos = append(repos, repoFilter)
}
})
return repos, negatedRepos

19
internal/search/query/validate.go
View File

@ -526,19 +526,14 @@ func parseYesNoOnly(s string) YesNoOnly {
}
func ContainsRefGlobs(q Q) bool {
containsRefGlobs := false
if repoFilterValues, _ := q.Repositories(); len(repoFilterValues) > 0 {
for _, v := range repoFilterValues {
repoRev := strings.SplitN(v, "@", 2)
if len(repoRev) == 1 { // no revision
continue
if repoFilters, _ := q.Repositories(); len(repoFilters) > 0 {
for _, r := range repoFilters {
for _, rev := range r.Revs {
if rev.HasRefGlob() {
return true
}
}
if ContainsNoGlobSyntax(repoRev[1]) {
continue
}
containsRefGlobs = true
break
}
}
return containsRefGlobs
return false
}

98
internal/search/repo_revs.go
View File

@ -8,51 +8,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/types"
)
// RevisionSpecifier represents either a revspec or a ref glob. At most one
// field is set. The default branch is represented by all fields being empty.
type RevisionSpecifier struct {
// RevSpec is a revision range specifier suitable for passing to git. See
// the manpage gitrevisions(7).
RevSpec string
// RefGlob is a reference glob to pass to git. See the documentation for
// "--glob" in git-log.
RefGlob string
// ExcludeRefGlob is a glob for references to exclude. See the
// documentation for "--exclude" in git-log.
ExcludeRefGlob string
}
func (r1 RevisionSpecifier) String() string {
if r1.ExcludeRefGlob != "" {
return "*!" + r1.ExcludeRefGlob
}
if r1.RefGlob != "" {
return "*" + r1.RefGlob
}
return r1.RevSpec
}
// Less compares two revspecOrRefGlob entities, suitable for use
// with sort.Slice()
//
// possibly-undesired: this results in treating an entity with
// no revspec, but a refGlob, as "earlier" than any revspec.
func (r1 RevisionSpecifier) Less(r2 RevisionSpecifier) bool {
if r1.RevSpec != r2.RevSpec {
return r1.RevSpec < r2.RevSpec
}
if r1.RefGlob != r2.RefGlob {
return r1.RefGlob < r2.RefGlob
}
return r1.ExcludeRefGlob < r2.ExcludeRefGlob
}
func (r1 RevisionSpecifier) HasRefGlob() bool {
return r1.RefGlob != "" || r1.ExcludeRefGlob != ""
}
// RepositoryRevisions specifies a repository and 0 or more revspecs and ref
// globs. If no revspecs and no ref globs are specified, then the
// repository's default branch is used.
@ -75,59 +30,6 @@ func (r *RepositoryRevisions) Equal(other *RepositoryRevisions) bool {
return reflect.DeepEqual(r.Repo, other.Repo) && reflect.DeepEqual(r.Revs, other.Revs)
}
// ParseRepositoryRevisions parses strings that refer to a repository and 0
// or more revspecs. The format is:
//
// repo@revs
//
// where repo is a repository regex and revs is a ':'-separated list of revspecs
// and/or ref globs. A ref glob is a revspec prefixed with '*' (which is not a
// valid revspec or ref itself; see `man git-check-ref-format`). The '@' and revs
// may be omitted to refer to the default branch.
//
// For example:
//
// - 'foo' refers to the 'foo' repo at the default branch
// - 'foo@bar' refers to the 'foo' repo and the 'bar' revspec.
// - 'foo@bar:baz:qux' refers to the 'foo' repo and 3 revspecs: 'bar', 'baz',
// and 'qux'.
// - 'foo@*bar' refers to the 'foo' repo and all refs matching the glob 'bar/*',
// because git interprets the ref glob 'bar' as being 'bar/*' (see `man git-log`
// section on the --glob flag)
func ParseRepositoryRevisions(repoAndOptionalRev string) (string, []RevisionSpecifier) {
i := strings.Index(repoAndOptionalRev, "@")
if i == -1 {
// return an empty slice to indicate that there's no revisions; callers
// have to distinguish between "none specified" and "default" to handle
// cases where two repo specs both match the same repository, and only one
// specifies a revspec, which normally implies "master" but in that case
// really means "didn't specify"
return repoAndOptionalRev, []RevisionSpecifier{}
}
repo := repoAndOptionalRev[:i]
var revs []RevisionSpecifier
for _, part := range strings.Split(repoAndOptionalRev[i+1:], ":") {
if part == "" {
continue
}
revs = append(revs, parseRev(part))
}
if len(revs) == 0 {
revs = []RevisionSpecifier{{RevSpec: ""}} // default branch
}
return repo, revs
}
func parseRev(spec string) RevisionSpecifier {
if strings.HasPrefix(spec, "*!") {
return RevisionSpecifier{ExcludeRefGlob: spec[2:]}
} else if strings.HasPrefix(spec, "*") {
return RevisionSpecifier{RefGlob: spec[1:]}
}
return RevisionSpecifier{RevSpec: spec}
}
// GitserverRepo is a convenience function to return the api.RepoName for
// r.Repo. The returned Repo will not have the URL set, only the name.
func (r *RepositoryRevisions) GitserverRepo() api.RepoName {

45
internal/search/repo_revs_test.go
View File

@ -1,45 +0,0 @@
package search
import (
"reflect"
"testing"
)
func TestParseRepositoryRevisions(t *testing.T) {
tests := map[string]struct {
repo string
revs []RevisionSpecifier
}{
"repo": {repo: "repo", revs: []RevisionSpecifier{}},
"repo@": {repo: "repo", revs: []RevisionSpecifier{{RevSpec: ""}}},
"repo@rev": {repo: "repo", revs: []RevisionSpecifier{{RevSpec: "rev"}}},
"repo@rev1:rev2": {repo: "repo", revs: []RevisionSpecifier{{RevSpec: "rev1"}, {RevSpec: "rev2"}}},
"repo@:rev1:": {repo: "repo", revs: []RevisionSpecifier{{RevSpec: "rev1"}}},
"repo@*glob": {repo: "repo", revs: []RevisionSpecifier{{RefGlob: "glob"}}},
"repo@rev1:*glob1:^rev2": {
repo: "repo",
revs: []RevisionSpecifier{{RevSpec: "rev1"}, {RefGlob: "glob1"}, {RevSpec: "^rev2"}},
},
"repo@rev1:*glob1:*!glob2:rev2:*glob3": {
repo: "repo",
revs: []RevisionSpecifier{
{RevSpec: "rev1"},
{RefGlob: "glob1"},
{ExcludeRefGlob: "glob2"},
{RevSpec: "rev2"},
{RefGlob: "glob3"},
},
},
}
for input, want := range tests {
t.Run(input, func(t *testing.T) {
repo, revs := ParseRepositoryRevisions(input)
if repo != want.repo {
t.Fatalf("got %+v, want %+v", repo, want.repo)
}
if !reflect.DeepEqual(revs, want.revs) {
t.Fatalf("got %+v, want %+v", revs, want.revs)
}
})
}
}

91
internal/search/repos/excluded_job_test.go Normal file
View File

@ -0,0 +1,91 @@
package repos
import (
"context"
"testing"
"github.com/stretchr/testify/require"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/search"
"github.com/sourcegraph/sourcegraph/internal/search/job"
"github.com/sourcegraph/sourcegraph/internal/search/query"
"github.com/sourcegraph/sourcegraph/internal/search/streaming"
)
func TestComputeExcludedJob(t *testing.T) {
tests := []struct {
name string
repoFilters []string
numArchived int
numForks int
expectedResult streaming.SearchEvent
}{
{
name: "compute excluded repos",
repoFilters: []string{"sourcegraph/.*"},
numArchived: 3,
numForks: 42,
expectedResult: streaming.SearchEvent{
Stats: streaming.Stats{
ExcludedArchived: 3,
ExcludedForks: 42,
},
},
},
{
name: "compute excluded repos with single matching repo",
repoFilters: []string{"^gitlab\\.com/sourcegraph/sourcegraph$"},
numArchived: 10,
numForks: 2,
expectedResult: streaming.SearchEvent{
Stats: streaming.Stats{
ExcludedArchived: 0,
ExcludedForks: 0,
},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
parsedFilters := make([]query.ParsedRepoFilter, len(tc.repoFilters))
for i, repoFilter := range tc.repoFilters {
parsedFilters[i] = query.ParseRepositoryRevisions(repoFilter)
}
repoStore := database.NewMockRepoStore()
repoStore.CountFunc.SetDefaultHook(func(_ context.Context, opt database.ReposListOptions) (int, error) {
// Verify that the include patterns passed to the DB match the repo filters
numFilters := len(parsedFilters)
require.Equal(t, numFilters, len(opt.IncludePatterns))
for i, repo := range opt.IncludePatterns {
require.Equal(t, parsedFilters[i].Repo, repo)
}
if opt.OnlyForks {
return tc.numForks, nil
} else if opt.OnlyArchived {
return tc.numArchived, nil
} else {
return numFilters, nil
}
})
db := database.NewMockDB()
db.ReposFunc.SetDefaultReturn(repoStore)
var result streaming.SearchEvent
streamCollector := streaming.StreamFunc(func(event streaming.SearchEvent) {
result = event
})
j := ComputeExcludedJob{RepoOpts: search.RepoOptions{RepoFilters: parsedFilters}}
alert, err := j.Run(context.Background(), job.RuntimeClients{DB: db}, streamCollector)
require.Nil(t, alert)
require.NoError(t, err)
require.Equal(t, tc.expectedResult, result)
})
}
}

52
internal/search/repos/repos.go
View File

@ -231,9 +231,9 @@ func (r *Resolver) Resolve(ctx context.Context, op search.RepoOptions) (_ Resolv
searchContextRepositoryRevisions = make(map[api.RepoID]RepoRevSpecs, len(scRepoRevs))
for _, repoRev := range scRepoRevs {
revSpecs := make([]search.RevisionSpecifier, 0, len(repoRev.Revs))
revSpecs := make([]query.RevisionSpecifier, 0, len(repoRev.Revs))
for _, rev := range repoRev.Revs {
revSpecs = append(revSpecs, search.RevisionSpecifier{RevSpec: rev})
revSpecs = append(revSpecs, query.RevisionSpecifier{RevSpec: rev})
}
searchContextRepositoryRevisions[repoRev.Repo.ID] = RepoRevSpecs{
Repo: repoRev.Repo,
@ -299,7 +299,7 @@ func (r *Resolver) associateReposWithRevs(
i, repo := i, repo
g.Go(func() {
var (
revs []search.RevisionSpecifier
revs []query.RevisionSpecifier
isMissing bool
)
@ -310,7 +310,7 @@ func (r *Resolver) associateReposWithRevs(
}
if len(revs) == 0 {
var clashingRevs []search.RevisionSpecifier
var clashingRevs []query.RevisionSpecifier
revs, clashingRevs = getRevsForMatchedRepo(repo.Name, includePatternRevs)
// if multiple specified revisions clash, report this usefully:
@ -393,7 +393,7 @@ func (r *Resolver) normalizeRefs(ctx context.Context, repoRevSpecs []RepoRevSpec
func (r *Resolver) normalizeRepoRefs(
ctx context.Context,
repo types.MinimalRepo,
revSpecs []search.RevisionSpecifier,
revSpecs []query.RevisionSpecifier,
reportMissing func(RepoRevSpecs),
) ([]string, error) {
revs := make([]string, 0, len(revSpecs))
@ -417,7 +417,7 @@ func (r *Resolver) normalizeRepoRefs(
if errors.Is(err, context.DeadlineExceeded) || errors.HasType(err, &gitdomain.BadCommitError{}) {
return nil, err
}
reportMissing(RepoRevSpecs{Repo: repo, Revs: []search.RevisionSpecifier{rev}})
reportMissing(RepoRevSpecs{Repo: repo, Revs: []query.RevisionSpecifier{rev}})
continue
}
revs = append(revs, rev.RevSpec)
@ -661,7 +661,7 @@ func (r *Resolver) filterRepoHasFileContent(
if errors.Is(err, context.DeadlineExceeded) || errors.HasType(err, &gitdomain.BadCommitError{}) {
return err
}
addMissing(RepoRevSpecs{Repo: repo, Revs: []search.RevisionSpecifier{{RevSpec: rev}}})
addMissing(RepoRevSpecs{Repo: repo, Revs: []query.RevisionSpecifier{{RevSpec: rev}}})
return nil
}
@ -787,7 +787,7 @@ func computeExcludedRepos(ctx context.Context, db database.DB, op search.RepoOpt
ExcludedRepos
}
if !op.ForkSet && !ExactlyOneRepo(includePatterns) {
if !op.ForkSet && !ExactlyOneRepo(op.RepoFilters) {
g.Go(func() error {
// 'fork:...' was not specified and Forks are excluded, find out
// which repos are excluded.
@ -807,7 +807,7 @@ func computeExcludedRepos(ctx context.Context, db database.DB, op search.RepoOpt
})
}
if !op.ArchivedSet && !ExactlyOneRepo(includePatterns) {
if !op.ArchivedSet && !ExactlyOneRepo(op.RepoFilters) {
g.Go(func() error {
// Archived...: was not specified and archives are excluded,
// find out which repos are excluded.
@ -834,11 +834,11 @@ func computeExcludedRepos(ctx context.Context, db database.DB, op search.RepoOpt
// delineated by regex anchors ^ and $. This function helps determine whether we
// should return results for a single repo regardless of whether it is a fork or
// archive.
func ExactlyOneRepo(repoFilters []string) bool {
func ExactlyOneRepo(repoFilters []query.ParsedRepoFilter) bool {
if len(repoFilters) == 1 {
filter, _ := search.ParseRepositoryRevisions(repoFilters[0])
if strings.HasPrefix(filter, "^") && strings.HasSuffix(filter, "$") {
filter := strings.TrimSuffix(strings.TrimPrefix(filter, "^"), "$")
repo := repoFilters[0].Repo
if strings.HasPrefix(repo, "^") && strings.HasSuffix(repo, "$") {
filter := strings.TrimSuffix(strings.TrimPrefix(repo, "^"), "$")
r, err := regexpsyntax.Parse(filter, regexpFlags)
if err != nil {
return false
@ -864,13 +864,13 @@ type ExcludedRepos struct {
// an actual map, because we want regexp matches, not identity matches.
type patternRevspec struct {
includePattern *regexp.Regexp
revs []search.RevisionSpecifier
revs []query.RevisionSpecifier
}
// given a repo name, determine whether it matched any patterns for which we have
// revspecs (or ref globs), and if so, return the matching/allowed ones.
func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []search.RevisionSpecifier, clashing []search.RevisionSpecifier) {
revLists := make([][]search.RevisionSpecifier, 0, len(pats))
func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []query.RevisionSpecifier, clashing []query.RevisionSpecifier) {
revLists := make([][]query.RevisionSpecifier, 0, len(pats))
for _, rev := range pats {
if rev.includePattern.MatchString(string(repo)) {
revLists = append(revLists, rev.revs)
@ -883,14 +883,14 @@ func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []
}
// no matches: we generate a dummy list containing only master
if len(revLists) == 0 {
matched = []search.RevisionSpecifier{{RevSpec: ""}}
matched = []query.RevisionSpecifier{{RevSpec: ""}}
return
}
// if two repo specs match, and both provided non-empty rev lists,
// we want their intersection, so we count the number of times we
// see a revision in the rev lists, and make sure it matches the number
// of rev lists
revCounts := make(map[search.RevisionSpecifier]int, len(revLists[0]))
revCounts := make(map[query.RevisionSpecifier]int, len(revLists[0]))
var aliveCount int
for i, revList := range revLists {
@ -904,7 +904,7 @@ func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []
}
if aliveCount > 0 {
matched = make([]search.RevisionSpecifier, 0, len(revCounts))
matched = make([]query.RevisionSpecifier, 0, len(revCounts))
for rev, seenCount := range revCounts {
if seenCount == len(revLists) {
matched = append(matched, rev)
@ -914,7 +914,7 @@ func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []
return
}
clashing = make([]search.RevisionSpecifier, 0, len(revCounts))
clashing = make([]query.RevisionSpecifier, 0, len(revCounts))
for rev := range revCounts {
clashing = append(clashing, rev)
}
@ -923,15 +923,15 @@ func getRevsForMatchedRepo(repo api.RepoName, pats []patternRevspec) (matched []
return
}
// findPatternRevs mutates the given list of include patterns to
// be a raw list of the repository name patterns we want, separating
// out their revision specs, if any.
func findPatternRevs(includePatterns []string) (outputPatterns []string, includePatternRevs []patternRevspec, err error) {
// findPatternRevs separates out each repo filter into its repository name
// pattern and its revision specs (if any). It validates each repository name
// pattern and applies some small optimizations.
func findPatternRevs(includePatterns []query.ParsedRepoFilter) (outputPatterns []string, includePatternRevs []patternRevspec, err error) {
outputPatterns = make([]string, 0, len(includePatterns))
includePatternRevs = make([]patternRevspec, 0, len(includePatterns))
for _, includePattern := range includePatterns {
repoPattern, revs := search.ParseRepositoryRevisions(includePattern)
repoPattern, revs := includePattern.Repo, includePattern.Revs
// Validate pattern now so the error message is more recognizable to the
// user
if _, err := regexp.Compile(repoPattern); err != nil {
@ -988,7 +988,7 @@ func (MissingRepoRevsError) Error() string { return "missing repo revs" }
type RepoRevSpecs struct {
Repo types.MinimalRepo
Revs []search.RevisionSpecifier
Revs []query.RevisionSpecifier
}
func (r *RepoRevSpecs) RevSpecs() []string {

51
internal/search/repos/repos_test.go
View File

@ -37,6 +37,14 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}
func toParsedRepoFilters(repoRevs ...string) []query.ParsedRepoFilter {
repoFilters := make([]query.ParsedRepoFilter, len(repoRevs))
for i, r := range repoRevs {
repoFilters[i] = query.ParseRepositoryRevisions(r)
}
return repoFilters
}
func TestRevisionValidation(t *testing.T) {
mockGitserver := gitserver.NewMockClient()
mockGitserver.ResolveRevisionFunc.SetDefaultHook(func(_ context.Context, _ api.RepoName, spec string, opt gitserver.ResolveRevisionOptions) (api.CommitID, error) {
@ -96,7 +104,7 @@ func TestRevisionValidation(t *testing.T) {
}},
wantMissingRepoRevisions: []RepoRevSpecs{{
Repo: types.MinimalRepo{Name: "repoFoo"},
Revs: []search.RevisionSpecifier{{
Revs: []query.RevisionSpecifier{{
RevSpec: "^revQux",
}},
}},
@ -138,7 +146,7 @@ func TestRevisionValidation(t *testing.T) {
db := database.NewMockDB()
db.ReposFunc.SetDefaultReturn(repos)
op := search.RepoOptions{RepoFilters: tt.repoFilters}
op := search.RepoOptions{RepoFilters: toParsedRepoFilters(tt.repoFilters...)}
repositoryResolver := NewResolver(logtest.Scoped(t), db, nil, nil, nil)
repositoryResolver.gitserver = mockGitserver
resolved, err := repositoryResolver.Resolve(context.Background(), op)
@ -167,8 +175,8 @@ func TestSearchRevspecs(t *testing.T) {
specs []string
repo string
err error
matched []search.RevisionSpecifier
clashing []search.RevisionSpecifier
matched []query.RevisionSpecifier
clashing []query.RevisionSpecifier
}
tests := []testCase{
@ -177,7 +185,7 @@ func TestSearchRevspecs(t *testing.T) {
specs: []string{"foo"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: ""}},
matched: []query.RevisionSpecifier{{RevSpec: ""}},
clashing: nil,
},
{
@ -185,7 +193,7 @@ func TestSearchRevspecs(t *testing.T) {
specs: []string{".*o@123456"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: "123456"}},
matched: []query.RevisionSpecifier{{RevSpec: "123456"}},
clashing: nil,
},
{
@ -193,7 +201,7 @@ func TestSearchRevspecs(t *testing.T) {
specs: []string{".*o@123456", "foo"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: "123456"}},
matched: []query.RevisionSpecifier{{RevSpec: "123456"}},
clashing: nil,
},
{
@ -201,7 +209,7 @@ func TestSearchRevspecs(t *testing.T) {
specs: []string{".*o", "foo@123456"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: "123456"}},
matched: []query.RevisionSpecifier{{RevSpec: "123456"}},
clashing: nil,
},
{
@ -210,14 +218,14 @@ func TestSearchRevspecs(t *testing.T) {
repo: "foo",
err: nil,
matched: nil,
clashing: []search.RevisionSpecifier{{RevSpec: "123456"}, {RevSpec: "234567"}},
clashing: []query.RevisionSpecifier{{RevSpec: "123456"}, {RevSpec: "234567"}},
},
{
descr: "overlapping revspecs",
specs: []string{".*o@a:b", "foo@b:c"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: "b"}},
matched: []query.RevisionSpecifier{{RevSpec: "b"}},
clashing: nil,
},
{
@ -225,7 +233,7 @@ func TestSearchRevspecs(t *testing.T) {
specs: []string{".*o@a:b:c", "foo@b:c:d"},
repo: "foo",
err: nil,
matched: []search.RevisionSpecifier{{RevSpec: "b"}, {RevSpec: "c"}},
matched: []query.RevisionSpecifier{{RevSpec: "b"}, {RevSpec: "c"}},
clashing: nil,
},
{
@ -239,7 +247,8 @@ func TestSearchRevspecs(t *testing.T) {
}
for _, test := range tests {
t.Run(test.descr, func(t *testing.T) {
_, pats, err := findPatternRevs(test.specs)
repoRevs := toParsedRepoFilters(test.specs...)
_, pats, err := findPatternRevs(repoRevs)
if err != nil {
if test.err == nil {
t.Errorf("unexpected error: '%s'", err)
@ -266,14 +275,16 @@ func TestSearchRevspecs(t *testing.T) {
func BenchmarkGetRevsForMatchedRepo(b *testing.B) {
b.Run("2 conflicting", func(b *testing.B) {
_, pats, _ := findPatternRevs([]string{".*o@123456", "foo@234567"})
repoRevs := toParsedRepoFilters(".*o@123456", "foo@234567")
_, pats, _ := findPatternRevs(repoRevs)
for i := 0; i < b.N; i++ {
_, _ = getRevsForMatchedRepo("foo", pats)
}
})
b.Run("multiple overlapping", func(b *testing.B) {
_, pats, _ := findPatternRevs([]string{".*o@a:b:c:d", "foo@b:c:d:e", "foo@c:d:e:f"})
repoRevs := toParsedRepoFilters(".*o@a:b:c:d", "foo@b:c:d:e", "foo@c:d:e:f")
_, pats, _ := findPatternRevs(repoRevs)
for i := 0; i < b.N; i++ {
_, _ = getRevsForMatchedRepo("foo", pats)
}
@ -314,7 +325,7 @@ func TestResolverIterator(t *testing.T) {
t.Fatal(err)
}
allAtRev, err := resolver.Resolve(ctx, search.RepoOptions{RepoFilters: []string{"foo/bar[0-4]@rev"}})
allAtRev, err := resolver.Resolve(ctx, search.RepoOptions{RepoFilters: toParsedRepoFilters("foo/bar[0-4]@rev")})
if err != nil {
t.Fatal(err)
}
@ -354,7 +365,7 @@ func TestResolverIterator(t *testing.T) {
name: "with limit 3 and fatal error",
opts: search.RepoOptions{
Limit: 3,
RepoFilters: []string{"foo/bar[0-5]@bad_commit"},
RepoFilters: toParsedRepoFilters("foo/bar[0-5]@bad_commit"),
},
err: &gitdomain.BadCommitError{},
pages: nil,
@ -363,7 +374,7 @@ func TestResolverIterator(t *testing.T) {
name: "with limit 3 and missing repo revs",
opts: search.RepoOptions{
Limit: 3,
RepoFilters: []string{"foo/bar[0-5]@rev"},
RepoFilters: toParsedRepoFilters("foo/bar[0-5]@rev"),
},
err: &MissingRepoRevsError{},
pages: []Resolved{
@ -372,7 +383,7 @@ func TestResolverIterator(t *testing.T) {
MissingRepoRevs: []RepoRevSpecs{
{
Repo: all.RepoRevs[0].Repo, // corresponds to foo/bar5
Revs: []search.RevisionSpecifier{
Revs: []query.RevisionSpecifier{
{
RevSpec: "rev",
},
@ -626,7 +637,7 @@ func TestRepoHasFileContent(t *testing.T) {
res := NewResolver(logtest.Scoped(t), db, gitserver.NewMockClient(), endpoint.Static("test"), mockZoekt)
resolved, err := res.Resolve(context.Background(), search.RepoOptions{
RepoFilters: []string{".*"},
RepoFilters: toParsedRepoFilters(".*"),
HasFileContent: tc.filters,
})
require.NoError(t, err)
@ -730,7 +741,7 @@ func TestRepoHasCommitAfter(t *testing.T) {
res := NewResolver(logtest.Scoped(t), db, nil, endpoint.Static("test"), nil)
res.gitserver = mockGitserver
resolved, err := res.Resolve(context.Background(), search.RepoOptions{
RepoFilters: []string{tc.nameFilter},
RepoFilters: toParsedRepoFilters(tc.nameFilter),
CommitAfter: tc.commitAfter,
})
require.Equal(t, tc.err, err)

12
internal/search/searchcontexts/search_contexts.go
View File

@ -209,9 +209,8 @@ func validateSearchContextQuery(contextQuery string) error {
return
}
repoRegex, revs := search.ParseRepositoryRevisions(value)
for _, rev := range revs {
repoRevs := query.ParseRepositoryRevisions(value)
for _, rev := range repoRevs.Revs {
if rev.HasRefGlob() {
errs = errors.Append(errs,
errors.Errorf("unsupported rev glob in search context query: %q", value))
@ -219,7 +218,7 @@ func validateSearchContextQuery(contextQuery string) error {
}
}
_, err := regexp.Compile(repoRegex)
_, err := regexp.Compile(repoRevs.Repo)
if err != nil {
errs = errors.Append(errs,
errors.Errorf("repo field regex %q is invalid: %v", value, err))
@ -489,13 +488,12 @@ func ParseRepoOpts(contextQuery string) ([]RepoOpts, error) {
}
for _, r := range repoFilters {
repoFilter, revs := search.ParseRepositoryRevisions(r)
for _, rev := range revs {
for _, rev := range r.Revs {
if !rev.HasRefGlob() {
rq.RevSpecs = append(rq.RevSpecs, rev.RevSpec)
}
}
rq.IncludePatterns = append(rq.IncludePatterns, repoFilter)
rq.IncludePatterns = append(rq.IncludePatterns, r.Repo)
}
qs = append(qs, rq)

7
internal/search/types.go
View File

@ -8,6 +8,8 @@ import (
"github.com/grafana/regexp"
otlog "github.com/opentracing/opentracing-go/log"
zoektquery "github.com/sourcegraph/zoekt/query"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/featureflag"
"github.com/sourcegraph/sourcegraph/internal/search/filter"
@ -17,7 +19,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/trace"
"github.com/sourcegraph/sourcegraph/internal/types"
"github.com/sourcegraph/sourcegraph/schema"
zoektquery "github.com/sourcegraph/zoekt/query"
)
// Inputs contains fields we set before kicking off search.
@ -365,7 +366,7 @@ func (f *Features) String() string {
// in their search query that affect which repos should be searched.
// When adding fields to this struct, be sure to update IsGlobal().
type RepoOptions struct {
RepoFilters []string
RepoFilters []query.ParsedRepoFilter
MinusRepoFilters []string
DescriptionPatterns []string
@ -404,7 +405,7 @@ func (op *RepoOptions) Tags() []otlog.Field {
}
if len(op.RepoFilters) > 0 {
add(trace.Strings("repoFilters", op.RepoFilters))
add(trace.Printf("repoFilters", "%v", op.RepoFilters))
}
if len(op.MinusRepoFilters) > 0 {
add(trace.Strings("minusRepoFilters", op.MinusRepoFilters))

9
internal/search/zoekt/indexed_search_test.go
View File

@ -18,6 +18,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/RoaringBitmap/roaring"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/search"
searchbackend "github.com/sourcegraph/sourcegraph/internal/search/backend"
@ -817,16 +818,16 @@ func TestContextWithoutDeadline_cancel(t *testing.T) {
func makeRepositoryRevisions(repos ...string) []*search.RepositoryRevisions {
r := make([]*search.RepositoryRevisions, len(repos))
for i, repospec := range repos {
repoName, revSpecs := search.ParseRepositoryRevisions(repospec)
revs := make([]string, 0, len(revSpecs))
for _, revSpec := range revSpecs {
repoRevs := query.ParseRepositoryRevisions(repospec)
revs := make([]string, 0, len(repoRevs.Revs))
for _, revSpec := range repoRevs.Revs {
revs = append(revs, revSpec.RevSpec)
}
if len(revs) == 0 {
// treat empty list as HEAD
revs = []string{"HEAD"}
}
r[i] = &search.RepositoryRevisions{Repo: mkRepos(repoName)[0], Revs: revs}
r[i] = &search.RepositoryRevisions{Repo: mkRepos(repoRevs.Repo)[0], Revs: revs}
}
return r
}