diff --git a/index/eval.go b/index/eval.go index 16b9adf..6f637f6 100644 --- a/index/eval.go +++ b/index/eval.go @@ -289,7 +289,7 @@ nextFileMatch: } fileMatch := zoekt.FileMatch{ - Repository: md.Name, + Repository: md.URL, RepositoryID: md.ID, RepositoryPriority: md.GetPriority(), FileName: string(d.fileName(nextDoc)), @@ -357,9 +357,12 @@ nextFileMatch: for _, md := range d.repoMetaData { r := md - addRepo(&res, &r) + // Key by URL: it is the unique repo identity and matches + // FileMatch.Repository (also set to the URL). Subrepos are keyed by + // their Name, matching FileMatch.SubRepositoryName. + addRepo(&res, r.URL, &r) for _, v := range r.SubRepoMap { - addRepo(&res, v) + addRepo(&res, v.Name, v) } } @@ -371,16 +374,16 @@ nextFileMatch: return &res, nil } -func addRepo(res *zoekt.SearchResult, repo *zoekt.Repository) { +func addRepo(res *zoekt.SearchResult, key string, repo *zoekt.Repository) { if res.RepoURLs == nil { res.RepoURLs = map[string]string{} } - res.RepoURLs[repo.Name] = repo.FileURLTemplate + res.RepoURLs[key] = repo.FileURLTemplate if res.LineFragments == nil { res.LineFragments = map[string]string{} } - res.LineFragments[repo.Name] = repo.LineFragmentTemplate + res.LineFragments[key] = repo.LineFragmentTemplate } // Gather matches from this document. The matches are returned in document @@ -547,7 +550,7 @@ func (d *indexData) List(ctx context.Context, q query.Q, opts *zoekt.ListOptions } include = func(rle *zoekt.RepoListEntry) bool { - _, ok := foundRepos[rle.Repository.Name] + _, ok := foundRepos[rle.Repository.URL] return ok } } diff --git a/index/index_test.go b/index/index_test.go index 988c88c..b5856ff 100644 --- a/index/index_test.go +++ b/index/index_test.go @@ -1640,11 +1640,12 @@ func TestRepoURL(t *testing.T) { sres := searchForTest(t, b, &query.Substring{Pattern: "bla"}) - if sres.RepoURLs["name"] != "file-url" { - t.Errorf("got RepoURLs %v, want {name: URL}", sres.RepoURLs) + // RepoURLs/LineFragments are keyed by the repo's URL (its unique identity). + if sres.RepoURLs["URL"] != "file-url" { + t.Errorf("got RepoURLs %v, want {URL: file-url}", sres.RepoURLs) } - if sres.LineFragments["name"] != "fragment" { - t.Errorf("got URLs %v, want {name: URL}", sres.LineFragments) + if sres.LineFragments["URL"] != "fragment" { + t.Errorf("got URLs %v, want {URL: fragment}", sres.LineFragments) } } @@ -3640,6 +3641,19 @@ func TestSearchTypeFileName(t *testing.T) { ) wantSingleMatch(res, "f2") }) + + // type:filematch is the default result granularity. The wrapper must be + // treated as a passthrough to the child instead of crashing the shard. + t.Run("TypeFileMatch", func(t *testing.T) { + res := searchForTest(t, b, + &query.Type{ + Type: query.TypeFileMatch, + Child: &query.Substring{Pattern: "needle"}, + }) + if len(res.Files) != 2 { + t.Fatalf("got %d file matches, want 2", len(res.Files)) + } + }) } func TestSearchTypeLanguage(t *testing.T) { diff --git a/index/matchtree.go b/index/matchtree.go index 6b0b91b..bd474a8 100644 --- a/index/matchtree.go +++ b/index/matchtree.go @@ -1057,7 +1057,11 @@ func (d *indexData) newMatchTree(q query.Q, opt matchTreeOpt) (matchTree, error) case *query.Type: if s.Type != query.TypeFileName { - break + // TypeFileMatch is the default result granularity, so the + // wrapper carries no extra match semantics: match the child + // directly. TypeRepo is normally stripped before reaching a + // shard, but handle it the same way to avoid a panic. + return d.newMatchTree(s.Child, opt) } ct, err := d.newMatchTree(s.Child, opt) diff --git a/internal/e2e/e2e_index_test.go b/internal/e2e/e2e_index_test.go index 4532633..3858e3a 100644 --- a/internal/e2e/e2e_index_test.go +++ b/internal/e2e/e2e_index_test.go @@ -50,6 +50,7 @@ func TestBasicIndexing(t *testing.T) { ShardMax: 1024, RepositoryDescription: zoekt.Repository{ Name: "repo", + URL: "repo", }, Parallelism: 2, SizeMax: 1 << 20, @@ -123,15 +124,16 @@ func TestBasicIndexing(t *testing.T) { // use retryTest to allow for the directory watcher to notice the meta // file retryTest(t, func(fatalf func(format string, args ...any)) { - // Add a .meta file for each shard with repo.Name set to - // "repo-mutated". We do this inside retry helper since we have noticed - // some flakiness on github CI. + // Add a .meta file for each shard with repo.URL set to + // "repo-mutated" (URL is the repo identity surfaced as + // FileMatch.Repository). We do this inside retry helper since we have + // noticed some flakiness on github CI. for _, p := range fs { repos, _, err := index.ReadMetadataPath(p) if err != nil { t.Fatal(err) } - repos[0].Name = "repo-mutated" + repos[0].URL = "repo-mutated" b, err := json.Marshal(repos[0]) if err != nil { t.Fatal(err) @@ -391,6 +393,7 @@ func TestUpdate(t *testing.T) { ShardMax: 1024, RepositoryDescription: zoekt.Repository{ Name: "repo", + URL: "repo", FileURLTemplate: "url", }, Parallelism: 2, @@ -429,6 +432,7 @@ func TestUpdate(t *testing.T) { opts.RepositoryDescription = zoekt.Repository{ Name: "repo2", + URL: "repo2", FileURLTemplate: "url2", } diff --git a/search/shards.go b/search/shards.go index 751390c..289fb99 100644 --- a/search/shards.go +++ b/search/shards.go @@ -835,11 +835,13 @@ func sendByRepository(result *zoekt.SearchResult, opts *zoekt.SearchOptions, sen return } - send := func(repoName string, a, b int, stats zoekt.Stats) { + send := func(repoURL string, a, b int, stats zoekt.Stats) { index.SortFiles(result.Files[a:b]) - filteredRepoURLs := map[string]string{repoName: result.RepoURLs[repoName]} - filteredLineFragments := map[string]string{repoName: result.LineFragments[repoName]} + // FileMatch.Repository holds the repo's unique key (its URL), which is + // also the key used for RepoURLs/LineFragments. + filteredRepoURLs := map[string]string{repoURL: result.RepoURLs[repoURL]} + filteredLineFragments := map[string]string{repoURL: result.LineFragments[repoURL]} // Filter RepoURLs and LineFragments to only those of repoName and its // subRepositories if there are subRepositories @@ -874,22 +876,22 @@ func sendByRepository(result *zoekt.SearchResult, opts *zoekt.SearchOptions, sen var startIndex, endIndex int curRepoID := result.Files[0].RepositoryID - curRepoName := result.Files[0].Repository + curRepoURL := result.Files[0].Repository fm := zoekt.FileMatch{} for endIndex, fm = range result.Files { if curRepoID != fm.RepositoryID { // Stats must stay aggregate-able, hence we sent the aggregate stats with the // last event. - send(curRepoName, startIndex, endIndex, zoekt.Stats{}) + send(curRepoURL, startIndex, endIndex, zoekt.Stats{}) startIndex = endIndex curRepoID = fm.RepositoryID - curRepoName = fm.Repository + curRepoURL = fm.Repository } } - send(curRepoName, startIndex, endIndex+1, result.Stats) + send(curRepoURL, startIndex, endIndex+1, result.Stats) } func observeMetrics(sr *zoekt.SearchResult) { @@ -1073,10 +1075,10 @@ func (ss *shardedSearcher) List(ctx context.Context, q query.Q, opts *zoekt.List agg.Stats.Add(&r.rl.Stats) for _, r := range r.rl.Repos { - prev, ok := uniq[r.Repository.Name] + prev, ok := uniq[r.Repository.URL] if !ok { cp := *r // We need to copy because we mutate r.Stats when merging duplicates - uniq[r.Repository.Name] = &cp + uniq[r.Repository.URL] = &cp } else { prev.Stats.Add(&r.Stats) } diff --git a/search/shards_test.go b/search/shards_test.go index ed1b7a7..56df700 100644 --- a/search/shards_test.go +++ b/search/shards_test.go @@ -429,6 +429,7 @@ func TestFilteringShardsByMeta(t *testing.T) { repo := &zoekt.Repository{ ID: uint32(i + 1), Name: repoName, + URL: repoName, Metadata: metadata, } @@ -645,11 +646,13 @@ func TestShardedSearcher_List(t *testing.T) { { ID: 1234, Name: "repo-a", + URL: "url-a", Branches: []zoekt.RepositoryBranch{{Name: "main"}, {Name: "dev"}}, RawConfig: map[string]string{"repoid": "1234"}, }, { Name: "repo-b", + URL: "url-b", Branches: []zoekt.RepositoryBranch{{Name: "main"}, {Name: "dev"}}, }, } @@ -938,7 +941,7 @@ func TestRawQuerySearch(t *testing.T) { var nextShardNum int addShard := func(repo string, rawConfig map[string]string, docs ...index.Document) { - r := &zoekt.Repository{Name: repo} + r := &zoekt.Repository{Name: repo, URL: repo} r.RawConfig = rawConfig b := testShardBuilder(t, r, docs...) shard := searcherForTest(t, b) diff --git a/web/e2e_test.go b/web/e2e_test.go index 03fc967..4f82ac1 100644 --- a/web/e2e_test.go +++ b/web/e2e_test.go @@ -307,7 +307,7 @@ func TestFormatJson(t *testing.T) { "json basic test", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -384,7 +384,7 @@ func TestContextLines(t *testing.T) { "no context doesn't return Before or After", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -404,7 +404,7 @@ func TestContextLines(t *testing.T) { "filename does not return Before or After", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -422,7 +422,7 @@ func TestContextLines(t *testing.T) { "context returns Before and After", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -444,7 +444,7 @@ func TestContextLines(t *testing.T) { "index at start returns After but no Before", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -465,7 +465,7 @@ func TestContextLines(t *testing.T) { "index at end returns Before but no After", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -486,7 +486,7 @@ func TestContextLines(t *testing.T) { "index with large context at end returns whole document", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -507,7 +507,7 @@ func TestContextLines(t *testing.T) { "index with large context at start returns whole document", FileMatch{ FileName: "f2", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f2", @@ -528,7 +528,7 @@ func TestContextLines(t *testing.T) { "context returns whitespaces lines", FileMatch{ FileName: "f4", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f4", @@ -550,7 +550,7 @@ func TestContextLines(t *testing.T) { "context returns new lines", FileMatch{ FileName: "f3", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f3", @@ -572,7 +572,7 @@ func TestContextLines(t *testing.T) { "context returns empty end line", FileMatch{ FileName: "f5", - Repo: "name", + Repo: "repo-url", Matches: []Match{ { FileName: "f5",