Skip to content

Commit 364a0e1

Browse files
authored
Fix panic when search yields repo and file matches in same repo (#385)
This fixes #382 by fixing a regression that was introduced by #361. When the search queries defined in a campaign spec yielded a repository match result and file match results in the same repository, in that order, the code would crash because of an uninitialized map. The code here fixes it by always initializing the map. I also added a test that reproduces the behaviour and documents how we turn multiple search results into a list of repositories.
1 parent a16b065 commit 364a0e1

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ All notable changes to `src-cli` are documented in this file.
1717

1818
### Fixed
1919

20+
- Fix a regression that was introduced by [#361](https://github.com/sourcegraph/src-cli/pull/361) and caused the "Resolving repositories" step of `src campaign [apply|preview]` to crash when the search query in the campaign spec yielded file matches and repository matches from the same repository.
21+
2022
### Removed
2123

2224
## 3.22.0

internal/campaigns/service.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,11 @@ func (sr *searchResult) UnmarshalJSON(data []byte) error {
535535
return nil
536536

537537
case "Repository":
538-
return json.Unmarshal(data, &sr.Repository)
538+
if err := json.Unmarshal(data, &sr.Repository); err != nil {
539+
return err
540+
}
541+
sr.Repository.FileMatches = map[string]bool{}
542+
return nil
539543

540544
default:
541545
return errors.Errorf("unknown GraphQL type %q", tn.Typename)

internal/campaigns/service_test.go

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
package campaigns
22

3-
import "testing"
3+
import (
4+
"bytes"
5+
"context"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
10+
"github.com/sourcegraph/src-cli/internal/api"
11+
)
412

513
func TestSetDefaultQueryCount(t *testing.T) {
614
for in, want := range map[string]string{
@@ -20,3 +28,94 @@ func TestSetDefaultQueryCount(t *testing.T) {
2028
})
2129
}
2230
}
31+
32+
func TestResolveRepositorySearch(t *testing.T) {
33+
mux := http.NewServeMux()
34+
mux.HandleFunc("/.api/graphql", func(w http.ResponseWriter, r *http.Request) {
35+
w.Header().Set("Content-Type", "application/json")
36+
w.Write([]byte(testResolveRepositorySearchResult))
37+
})
38+
39+
ts := httptest.NewServer(mux)
40+
defer ts.Close()
41+
42+
var clientBuffer bytes.Buffer
43+
client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer})
44+
45+
svc := &Service{client: client}
46+
47+
repos, err := svc.resolveRepositorySearch(context.Background(), "sourcegraph reconciler or src-cli")
48+
if err != nil {
49+
t.Fatalf("unexpected error: %s", err)
50+
}
51+
52+
// We want multiple FileMatches in the same repository to result in one repository
53+
// and Repository and FileMatches should be folded into the same repository.
54+
if have, want := len(repos), 3; have != want {
55+
t.Fatalf("wrong number of repos. want=%d, have=%d", want, have)
56+
}
57+
}
58+
59+
const testResolveRepositorySearchResult = `{
60+
"data": {
61+
"search": {
62+
"results": {
63+
"results": [
64+
{
65+
"__typename": "FileMatch",
66+
"file": { "path": "website/src/components/PricingTable.tsx" },
67+
"repository": {
68+
"id": "UmVwb3NpdG9yeTo0MDc=",
69+
"name": "github.com/sd9/about",
70+
"url": "/github.com/sd9/about",
71+
"externalRepository": { "serviceType": "github" },
72+
"defaultBranch": { "name": "refs/heads/master", "target": { "oid": "1576f235209fbbfc918129db35f3d108347b74cb" } }
73+
}
74+
},
75+
{
76+
"__typename": "Repository",
77+
"id": "UmVwb3NpdG9yeToxOTM=",
78+
"name": "github.com/sd9/src-cli",
79+
"url": "/github.com/sd9/src-cli",
80+
"externalRepository": { "serviceType": "github" },
81+
"defaultBranch": { "name": "refs/heads/master", "target": { "oid": "21dd58b08d64620942401b5543f5b0d33498bacb" } }
82+
},
83+
{
84+
"__typename": "FileMatch",
85+
"file": { "path": "cmd/src/api.go" },
86+
"repository": {
87+
"id": "UmVwb3NpdG9yeToxOTM=",
88+
"name": "github.com/sd9/src-cli",
89+
"url": "/github.com/sd9/src-cli",
90+
"externalRepository": { "serviceType": "github" },
91+
"defaultBranch": { "name": "refs/heads/master", "target": { "oid": "21dd58b08d64620942401b5543f5b0d33498bacb" } }
92+
}
93+
},
94+
{
95+
"__typename": "FileMatch",
96+
"file": { "path": "client/web/src/components/externalServices/externalServices.tsx" },
97+
"repository": {
98+
"id": "UmVwb3NpdG9yeTo2",
99+
"name": "github.com/sourcegraph/sourcegraph",
100+
"url": "/github.com/sourcegraph/sourcegraph",
101+
"externalRepository": { "serviceType": "github" },
102+
"defaultBranch": { "name": "refs/heads/main", "target": { "oid": "e612c34f2e27005928ff3dfdd8e5ead5a0cb1526" } }
103+
}
104+
},
105+
{
106+
"__typename": "FileMatch",
107+
"file": { "path": "cmd/frontend/internal/httpapi/src_cli.go" },
108+
"repository": {
109+
"id": "UmVwb3NpdG9yeTo2",
110+
"name": "github.com/sourcegraph/sourcegraph",
111+
"url": "/github.com/sourcegraph/sourcegraph",
112+
"externalRepository": { "serviceType": "github" },
113+
"defaultBranch": { "name": "refs/heads/main", "target": { "oid": "e612c34f2e27005928ff3dfdd8e5ead5a0cb1526" } }
114+
}
115+
}
116+
]
117+
}
118+
}
119+
}
120+
}
121+
`

0 commit comments

Comments
 (0)