Skip to content

Commit

Permalink
remove repo_urls and line_fragments from protobuf defintion
Browse files Browse the repository at this point in the history
  • Loading branch information
ggilmore committed Aug 15, 2023
1 parent a4445ac commit 4f2bc6e
Show file tree
Hide file tree
Showing 6 changed files with 497 additions and 579 deletions.
23 changes: 12 additions & 11 deletions api_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func (p *Progress) ToProto() *proto.Progress {
}
}

func SearchResultFromProto(p *proto.SearchResponse) *SearchResult {
func SearchResultFromProto(p *proto.SearchResponse, repoURls, lineFragments map[string]string) *SearchResult {
if p == nil {
return nil
}
Expand All @@ -356,11 +356,13 @@ func SearchResultFromProto(p *proto.SearchResponse) *SearchResult {
}

return &SearchResult{
Stats: StatsFromProto(p.GetStats()),
Progress: ProgressFromProto(p.GetProgress()),
Files: files,
RepoURLs: p.RepoUrls,
LineFragments: p.LineFragments,
Stats: StatsFromProto(p.GetStats()),
Progress: ProgressFromProto(p.GetProgress()),

Files: files,

RepoURLs: repoURls,
LineFragments: lineFragments,
}
}

Expand All @@ -375,11 +377,10 @@ func (sr *SearchResult) ToProto() *proto.SearchResponse {
}

return &proto.SearchResponse{
Stats: sr.Stats.ToProto(),
Progress: sr.Progress.ToProto(),
Files: files,
RepoUrls: sr.RepoURLs,
LineFragments: sr.LineFragments,
Stats: sr.Stats.ToProto(),
Progress: sr.Progress.ToProto(),

Files: files,
}
}

Expand Down
13 changes: 11 additions & 2 deletions api_proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,17 @@ func TestProtoRoundtrip(t *testing.T) {

t.Run("SearchResult", func(t *testing.T) {
f := func(f1 *SearchResult) bool {
var repoURLs map[string]string
var lineFragments map[string]string

if f1 != nil {
repoURLs = f1.RepoURLs
lineFragments = f1.LineFragments
}

p1 := f1.ToProto()
f2 := SearchResultFromProto(p1)
f2 := SearchResultFromProto(p1, repoURLs, lineFragments)

return reflect.DeepEqual(f1, f2)
}
if err := quick.Check(f, nil); err != nil {
Expand Down Expand Up @@ -397,7 +406,7 @@ var (
}()

// The non-proto struct representation of the search result
exampleSearchResultGo = SearchResultFromProto(exampleSearchResultProto)
exampleSearchResultGo = SearchResultFromProto(exampleSearchResultProto, nil, nil)
)

func BenchmarkGobRoundtrip(b *testing.B) {
Expand Down
23 changes: 2 additions & 21 deletions grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,30 +108,11 @@ func gRPCChunkSender(ss v1.WebserverService_StreamSearchServer) zoekt.Sender {
}
}

// We need to ensure that we send the repository URLs and LineFragments for only the repositories
// actually mentioned in this chunk of files.
repoURLs := make(map[string]string, len(result.GetRepoUrls()))
lineFragments := make(map[string]string, len(result.GetLineFragments()))

for _, file := range filesChunk {
repository := file.GetRepository()

if url, ok := result.GetRepoUrls()[repository]; ok {
repoURLs[repository] = url
}

if fragment, ok := result.GetLineFragments()[repository]; ok {
lineFragments[repository] = fragment
}
}

response := &v1.SearchResponse{
Files: filesChunk,
Files: filesChunk,

Stats: stats,
Progress: progress,

RepoUrls: repoURLs,
LineFragments: lineFragments,
}

return ss.Send(response)
Expand Down
31 changes: 0 additions & 31 deletions grpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,6 @@ func TestFuzzGRPCChunkSender(t *testing.T) {
// Setup: we constrain the input to be a valid zoekt.SearchResult by ensuring that the
// RepoURLs and LineFragment fields are derived from the FileMatches
// (instead of being random maps from quick.Check)

input.RepoURLs = make(map[string]string)
input.LineFragments = make(map[string]string)
for _, file := range input.Files {
input.RepoURLs[file.Repository] = fmt.Sprintf("https://github.com/%s", file.Repository)
input.LineFragments[file.Repository] = fmt.Sprintf("line fragment for %s", file.Repository)
}

clientStream, serverStream := newPairedSearchStream(t)
sender := gRPCChunkSender(serverStream)

Expand Down Expand Up @@ -158,9 +150,6 @@ func TestFuzzGRPCChunkSender(t *testing.T) {
"progress", // progress is tested above
"stats", // aggregated stats are tested below
"files", // files are tested separately

"repo_urls", // tested below
"line_fragments", // tested below
),
}

Expand All @@ -170,21 +159,11 @@ func TestFuzzGRPCChunkSender(t *testing.T) {
}

receivedStats := &zoekt.Stats{}
receivedRepoURLs := make(map[string]string)
receivedLineFragments := make(map[string]string)

var receivedFileMatches []*v1.FileMatch
for _, r := range allResponses {
receivedStats.Add(zoekt.StatsFromProto(r.GetStats()))
receivedFileMatches = append(receivedFileMatches, r.GetFiles()...)

for k, v := range r.GetRepoUrls() {
receivedRepoURLs[k] = v
}

for k, v := range r.GetLineFragments() {
receivedLineFragments[k] = v
}
}

// Check to make sure that we get one set of stats back
Expand All @@ -197,16 +176,6 @@ func TestFuzzGRPCChunkSender(t *testing.T) {
return fmt.Errorf("unexpected difference in stats (-want +got):\n%s", diff)
}

// Check to make sure that we get the expected set of repo URLs back
if diff := cmp.Diff(expectedResult.GetRepoUrls(), receivedRepoURLs, cmpopts.EquateEmpty()); diff != "" {
return fmt.Errorf("unexpected difference in repo URLs (-want +got):\n%s", diff)
}

// Check to make sure that we get the expected set of line fragments back
if diff := cmp.Diff(expectedResult.GetLineFragments(), receivedLineFragments, cmpopts.EquateEmpty()); diff != "" {
return fmt.Errorf("unexpected difference in line fragments (-want +got):\n%s", diff)
}

// Check to make sure that we get the same set of file matches back
if diff := cmp.Diff(expectedResult.GetFiles(), receivedFileMatches,
protocmp.Transform(), cmpopts.EquateEmpty()); diff != "" {
Expand Down
Loading

0 comments on commit 4f2bc6e

Please sign in to comment.