search: return a result per branch in indexed results (#12249)

If a file is the same in two branches and we have results on it, our
indexed search currently will only return one result. This would be
misleading in the UI since the user would not know the other branches
have results.

This change will copy how we do it for non-indexed search and just
duplicate results (but with different input revs). In future we should
dedup results and display this information more compactly.

cmd/frontend/graphqlbackend/zoekt.go

@@ -324,9 +324,9 @@ func zoektSearch(ctx context.Context, args *search.TextParameters, repos *indexe
 		limitHit = true
 	}
 
-	matches := make([]*FileMatchResolver, len(resp.Files))
+	matches := make([]*FileMatchResolver, 0, len(resp.Files))
 	repoResolvers := make(RepositoryResolverCache)
-	for i, file := range resp.Files {
+	for _, file := range resp.Files {
 		fileLimitHit := false
 		if len(file.LineMatches) > maxLineMatches {
 			file.LineMatches = file.LineMatches[:maxLineMatches]
@@ -334,35 +334,38 @@ func zoektSearch(ctx context.Context, args *search.TextParameters, repos *indexe
 			limitHit = true
 		}
 
-		repo, inputRev := repos.GetRepoInputRev(&file)
+		repo, inputRevs := repos.GetRepoInputRev(&file)
 		repoResolver := repoResolvers[repo.Name]
 		if repoResolver == nil {
 			repoResolver = &RepositoryResolver{repo: repo}
 			repoResolvers[repo.Name] = repoResolver
 		}
 
-		// symbols is set in symbols search, lines in text search.
-		var (
-			symbols    []*searchSymbolResult
-			lines      []*lineMatch
-			matchCount int
-		)
+		var lines []*lineMatch
+		var matchCount int
 		if typ != symbolRequest {
 			lines, matchCount = zoektFileMatchToLineMatches(maxLineFragmentMatches, &file)
-		} else {
-			symbols = zoektFileMatchToSymbolResults(repoResolver, inputRev, &file)
 		}
 
-		matches[i] = &FileMatchResolver{
-			JPath:        file.FileName,
-			JLineMatches: lines,
-			JLimitHit:    fileLimitHit,
-			MatchCount:   matchCount, // We do not use resp.MatchCount because it counts the number of lines matched, not the number of fragments.
-			uri:          fileMatchURI(repo.Name, inputRev, file.FileName),
-			symbols:      symbols,
-			Repo:         repoResolver,
-			CommitID:     api.CommitID(file.Version),
-			InputRev:     &inputRev,
+		for _, inputRev := range inputRevs {
+			inputRev := inputRev // copy so we can take the pointer
+
+			var symbols []*searchSymbolResult
+			if typ == symbolRequest {
+				symbols = zoektFileMatchToSymbolResults(repoResolver, inputRev, &file)
+			}
+
+			matches = append(matches, &FileMatchResolver{
+				JPath:        file.FileName,
+				JLineMatches: lines,
+				JLimitHit:    fileLimitHit,
+				MatchCount:   matchCount, // We do not use resp.MatchCount because it counts the number of lines matched, not the number of fragments.
+				uri:          fileMatchURI(repo.Name, inputRev, file.FileName),
+				symbols:      symbols,
+				Repo:         repoResolver,
+				CommitID:     api.CommitID(file.Version),
+				InputRev:     &inputRev,
+			})
 		}
 	}
 
@@ -658,24 +661,24 @@ func (rb *indexedRepoRevs) Add(reporev *search.RepositoryRevisions, repo *zoekt.
 }
 
 // GetRepoInputRev returns the repo and inputRev associated with file.
-func (rb *indexedRepoRevs) GetRepoInputRev(file *zoekt.FileMatch) (repo *types.Repo, inputRev string) {
+func (rb *indexedRepoRevs) GetRepoInputRev(file *zoekt.FileMatch) (repo *types.Repo, inputRevs []string) {
 	repoRev := rb.repoRevs[file.Repository]
 
-	// TODO(keegancsmith) We need to handle results across branches for the
-	// same file. Options:
-	// 1. a filematch per file.Branches
-	// 2. update result schema to have multiple uris.
-	//
-	// For now we only show one result for simplicity.
-	branch := file.Branches[0]
-	for i, b := range rb.repoBranches[file.Repository] {
-		if branch == b {
-			// RevSpec is guaranteed to be explicit via zoektIndexedRepos
-			return repoRev.Repo, repoRev.Revs[i].RevSpec
+	inputRevs = make([]string, 0, len(file.Branches))
+	for _, branch := range file.Branches {
+		for i, b := range rb.repoBranches[file.Repository] {
+			if branch == b {
+				// RevSpec is guaranteed to be explicit via zoektIndexedRepos
+				inputRevs = append(inputRevs, repoRev.Revs[i].RevSpec)
+			}
 		}
 	}
 
-	// Did not find a match. This is unexpected, but we can fallback to
-	// file.Version to generate correct links.
-	return repoRev.Repo, file.Version
+	if len(inputRevs) == 0 {
+		// Did not find a match. This is unexpected, but we can fallback to
+		// file.Version to generate correct links.
+		inputRevs = append(inputRevs, file.Version)
+	}
+
+	return repoRev.Repo, inputRevs
 }

cmd/frontend/graphqlbackend/zoekt_test.go

@@ -81,7 +81,7 @@ func TestIndexedSearch(t *testing.T) {
 	zoektRepos := []*zoekt.RepoListEntry{{
 		Repository: zoekt.Repository{
 			Name:     "foo/bar",
-			Branches: []zoekt.RepositoryBranch{{Name: "HEAD", Version: "barHEADSHA"}, {Name: "dev", Version: "bardevSHA"}},
+			Branches: []zoekt.RepositoryBranch{{Name: "HEAD", Version: "barHEADSHA"}, {Name: "dev", Version: "bardevSHA"}, {Name: "main", Version: "barmainSHA"}},
 		},
 	}, {
 		Repository: zoekt.Repository{
@@ -91,13 +91,14 @@ func TestIndexedSearch(t *testing.T) {
 	}}
 
 	tests := []struct {
-		name              string
-		args              args
-		wantMatchCount    int
-		wantMatchURLs     []string
-		wantLimitHit      bool
-		wantReposLimitHit map[string]struct{}
-		wantErr           bool
+		name               string
+		args               args
+		wantMatchCount     int
+		wantMatchURLs      []string
+		wantMatchInputRevs []string
+		wantLimitHit       bool
+		wantReposLimitHit  map[string]struct{}
+		wantErr            bool
 	}{
 		{
 			name: "no matches",
@@ -190,6 +191,10 @@ func TestIndexedSearch(t *testing.T) {
 				"git://foo/bar#baz.go",
 				"git://foo/foobar#baz.go",
 			},
+			wantMatchInputRevs: []string{
+				"",
+				"",
+			},
 			wantErr: false,
 		},
 		{
@@ -197,18 +202,19 @@ func TestIndexedSearch(t *testing.T) {
 			args: args{
 				ctx:             context.Background(),
 				query:           &search.TextPatternInfo{FileMatchLimit: 100},
-				repos:           makeRepositoryRevisions("foo/bar@HEAD:dev"),
+				repos:           makeRepositoryRevisions("foo/bar@HEAD:dev:main"),
 				useFullDeadline: false,
 				results: []zoekt.FileMatch{
 					{
 						Repository: "foo/bar",
-						Branches:   []string{"HEAD"},
-						FileName:   "baz.go",
+						// baz.go is the same in HEAD and dev
+						Branches: []string{"HEAD", "dev"},
+						FileName: "baz.go",
 					},
 					{
 						Repository: "foo/bar",
 						Branches:   []string{"dev"},
-						FileName:   "baz.go",
+						FileName:   "bam.go",
 					},
 				},
 				since: func(time.Time) time.Duration { return 0 },
@@ -218,6 +224,12 @@ func TestIndexedSearch(t *testing.T) {
 			wantMatchURLs: []string{
 				"git://foo/bar?HEAD#baz.go",
 				"git://foo/bar?dev#baz.go",
+				"git://foo/bar?dev#bam.go",
+			},
+			wantMatchInputRevs: []string{
+				"HEAD",
+				"dev",
+				"dev",
 			},
 			wantErr: false,
 		},
@@ -264,13 +276,20 @@ func TestIndexedSearch(t *testing.T) {
 
 			var gotMatchCount int
 			var gotMatchURLs []string
+			var gotMatchInputRevs []string
 			for _, m := range gotFm {
 				gotMatchCount += m.MatchCount
 				gotMatchURLs = append(gotMatchURLs, m.Resource())
+				if m.InputRev != nil {
+					gotMatchInputRevs = append(gotMatchInputRevs, *m.InputRev)
+				}
 			}
 			if diff := cmp.Diff(tt.wantMatchURLs, gotMatchURLs); diff != "" {
 				t.Errorf("match URLs mismatch (-want +got):\n%s", diff)
 			}
+			if diff := cmp.Diff(tt.wantMatchInputRevs, gotMatchInputRevs); diff != "" {
+				t.Errorf("match InputRevs mismatch (-want +got):\n%s", diff)
+			}
 			if gotMatchCount != tt.wantMatchCount {
 				t.Errorf("gotMatchCount = %v, want %v", gotMatchCount, tt.wantMatchCount)
 			}