Skip to content

Commit 0ad153f

Browse files
mislavmislav
committed
Separate payload structs for REST vs GraphQL repo create
This enforces strict separation between serialization structs used for repository creation payload with respect to whether GraphQL or REST was used. Before, a field added to a GraphQL payload would leak to REST payload (and vice versa).
1 parent e600ce0 commit 0ad153f

File tree

5 files changed

+505
-252
lines changed

5 files changed

+505
-252
lines changed

‎pkg/cmd/repo/create/create.go‎

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -255,15 +255,27 @@ func createRun(opts *CreateOptions) error {
255255
repoToCreate = ghrepo.NewWithHost("", opts.Name, host)
256256
}
257257

258+
input := repoCreateInput{
259+
Name: repoToCreate.RepoName(),
260+
Visibility: visibility,
261+
OwnerLogin: repoToCreate.RepoOwner(),
262+
TeamSlug: opts.Team,
263+
Description: opts.Description,
264+
HomepageURL: opts.Homepage,
265+
HasIssuesEnabled: opts.EnableIssues,
266+
HasWikiEnabled: opts.EnableWiki,
267+
GitIgnoreTemplate: gitIgnoreTemplate,
268+
LicenseTemplate: repoLicenseTemplate,
269+
}
270+
271+
httpClient, err := opts.HttpClient()
272+
if err != nil {
273+
return err
274+
}
275+
258276
var templateRepoMainBranch string
259-
// Find template repo ID
260277
if opts.Template != "" {
261-
httpClient, err := opts.HttpClient()
262-
if err != nil {
263-
return err
264-
}
265-
266-
var toClone ghrepo.Interface
278+
var templateRepo ghrepo.Interface
267279
apiClient := api.NewClientFromHTTP(httpClient)
268280

269281
cloneURL := opts.Template
@@ -274,48 +286,30 @@ func createRun(opts *CreateOptions) error {
274286
}
275287
cloneURL = currentUser + "/" + cloneURL
276288
}
277-
toClone, err = ghrepo.FromFullName(cloneURL)
289+
templateRepo, err = ghrepo.FromFullName(cloneURL)
278290
if err != nil {
279291
return fmt.Errorf("argument error: %w", err)
280292
}
281293

282-
repo, err := api.GitHubRepo(apiClient, toClone)
294+
repo, err := api.GitHubRepo(apiClient, templateRepo)
283295
if err != nil {
284296
return err
285297
}
286298

287-
opts.Template = repo.ID
299+
input.TemplateRepositoryID = repo.ID
288300
templateRepoMainBranch = repo.DefaultBranchRef.Name
289301
}
290302

291-
input := repoCreateInput{
292-
Name: repoToCreate.RepoName(),
293-
Visibility: visibility,
294-
OwnerID: repoToCreate.RepoOwner(),
295-
TeamID: opts.Team,
296-
Description: opts.Description,
297-
HomepageURL: opts.Homepage,
298-
HasIssuesEnabled: opts.EnableIssues,
299-
HasWikiEnabled: opts.EnableWiki,
300-
GitIgnoreTemplate: gitIgnoreTemplate,
301-
LicenseTemplate: repoLicenseTemplate,
302-
}
303-
304-
httpClient, err := opts.HttpClient()
305-
if err != nil {
306-
return err
307-
}
308-
309303
createLocalDirectory := opts.ConfirmSubmit
310304
if !opts.ConfirmSubmit {
311-
opts.ConfirmSubmit, err = confirmSubmission(input.Name, input.OwnerID, inLocalRepo)
305+
opts.ConfirmSubmit, err = confirmSubmission(input.Name, input.OwnerLogin, inLocalRepo)
312306
if err != nil {
313307
return err
314308
}
315309
}
316310

317311
if opts.ConfirmSubmit {
318-
repo, err := repoCreate(httpClient, repoToCreate.RepoHost(), input, opts.Template)
312+
repo, err := repoCreate(httpClient, repoToCreate.RepoHost(), input)
319313
if err != nil {
320314
return err
321315
}

‎pkg/cmd/repo/create/create_test.go‎

Lines changed: 16 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -606,27 +606,19 @@ func TestRepoCreate_WithGitIgnore(t *testing.T) {
606606
assert.Equal(t, "", output.String())
607607
assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr())
608608

609-
var reqBody struct {
610-
Name string
611-
Visibility string
612-
OwnerId string
613-
LicenseTemplate string
614-
}
615-
616609
if len(reg.Requests) != 3 {
617610
t.Fatalf("expected 3 HTTP request, got %d", len(reg.Requests))
618611
}
619612

620-
bodyBytes, _ := ioutil.ReadAll(reg.Requests[2].Body)
621-
_ = json.Unmarshal(bodyBytes, &reqBody)
622-
if repoName := reqBody.Name; repoName != "REPO" {
623-
t.Errorf("expected %q, got %q", "REPO", repoName)
624-
}
625-
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
626-
t.Errorf("expected %q, got %q", "private", repoVisibility)
613+
reqBody := make(map[string]interface{})
614+
dec := json.NewDecoder(reg.Requests[2].Body)
615+
assert.NoError(t, dec.Decode(&reqBody))
616+
617+
if gitignore := reqBody["gitignore_template"]; gitignore != "Go" {
618+
t.Errorf("expected %q, got %q", "Go", gitignore)
627619
}
628-
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
629-
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
620+
if license := reqBody["license_template"]; license != nil {
621+
t.Errorf("expected %v, got %v", nil, license)
630622
}
631623
}
632624

@@ -671,7 +663,7 @@ func TestRepoCreate_WithBothGitIgnoreLicense(t *testing.T) {
671663
as.Stub([]*prompt.QuestionStub{
672664
{
673665
Name: "chooseLicense",
674-
Value: "GNU Affero General Public License v3.0",
666+
Value: "GNU Lesser General Public License v3.0",
675667
},
676668
})
677669

@@ -705,115 +697,18 @@ func TestRepoCreate_WithBothGitIgnoreLicense(t *testing.T) {
705697
assert.Equal(t, "", output.String())
706698
assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr())
707699

708-
var reqBody struct {
709-
Name string
710-
Visibility string
711-
OwnerId string
712-
LicenseTemplate string
713-
}
714-
715700
if len(reg.Requests) != 4 {
716701
t.Fatalf("expected 4 HTTP request, got %d", len(reg.Requests))
717702
}
718703

719-
bodyBytes, _ := ioutil.ReadAll(reg.Requests[3].Body)
720-
_ = json.Unmarshal(bodyBytes, &reqBody)
721-
if repoName := reqBody.Name; repoName != "REPO" {
722-
t.Errorf("expected %q, got %q", "REPO", repoName)
723-
}
724-
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
725-
t.Errorf("expected %q, got %q", "private", repoVisibility)
726-
}
727-
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
728-
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
729-
}
730-
}
731-
732-
func TestRepoCreate_WithGitIgnore_Org(t *testing.T) {
733-
cs, cmdTeardown := run.Stub()
734-
defer cmdTeardown(t)
704+
reqBody := make(map[string]interface{})
705+
dec := json.NewDecoder(reg.Requests[3].Body)
706+
assert.NoError(t, dec.Decode(&reqBody))
735707

736-
cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "")
737-
cs.Register(`git rev-parse --show-toplevel`, 0, "")
738-
739-
as, surveyTearDown := prompt.InitAskStubber()
740-
defer surveyTearDown()
741-
742-
as.Stub([]*prompt.QuestionStub{
743-
{
744-
Name: "repoVisibility",
745-
Value: "PRIVATE",
746-
},
747-
})
748-
749-
as.Stub([]*prompt.QuestionStub{
750-
{
751-
Name: "addGitIgnore",
752-
Value: true,
753-
},
754-
})
755-
756-
as.Stub([]*prompt.QuestionStub{
757-
{
758-
Name: "chooseGitIgnore",
759-
Value: "Go",
760-
},
761-
})
762-
763-
as.Stub([]*prompt.QuestionStub{
764-
{
765-
Name: "addLicense",
766-
Value: false,
767-
},
768-
})
769-
770-
as.Stub([]*prompt.QuestionStub{
771-
{
772-
Name: "confirmSubmit",
773-
Value: true,
774-
},
775-
})
776-
777-
reg := &httpmock.Registry{}
778-
reg.Register(
779-
httpmock.REST("GET", "users/OWNER"),
780-
httpmock.StringResponse(`{ "node_id": "OWNERID", "type":"Organization" }`))
781-
reg.Register(
782-
httpmock.REST("GET", "gitignore/templates"),
783-
httpmock.StringResponse(`["Actionscript","Android","AppceleratorTitanium","Autotools","Bancha","C","C++","Go"]`))
784-
reg.Register(
785-
httpmock.REST("POST", "orgs/OWNER/repos"),
786-
httpmock.StringResponse(`{"name":"REPO", "owner":{"login": "OWNER"}, "html_url":"https://github.com/OWNER/REPO"}`))
787-
httpClient := &http.Client{Transport: reg}
788-
789-
output, err := runCommand(httpClient, "OWNER/REPO", true)
790-
if err != nil {
791-
t.Errorf("error running command `repo create`: %v", err)
792-
}
793-
794-
assert.Equal(t, "", output.String())
795-
assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr())
796-
797-
var reqBody struct {
798-
Name string
799-
Visibility string
800-
OwnerId string
801-
LicenseTemplate string
708+
if gitignore := reqBody["gitignore_template"]; gitignore != "Go" {
709+
t.Errorf("expected %q, got %q", "Go", gitignore)
802710
}
803-
804-
if len(reg.Requests) != 3 {
805-
t.Fatalf("expected 3 HTTP request, got %d", len(reg.Requests))
806-
}
807-
808-
bodyBytes, _ := ioutil.ReadAll(reg.Requests[2].Body)
809-
_ = json.Unmarshal(bodyBytes, &reqBody)
810-
if repoName := reqBody.Name; repoName != "REPO" {
811-
t.Errorf("expected %q, got %q", "REPO", repoName)
812-
}
813-
if repoVisibility := reqBody.Visibility; repoVisibility != "private" {
814-
t.Errorf("expected %q, got %q", "private", repoVisibility)
815-
}
816-
if ownerId := reqBody.OwnerId; ownerId != "OWNERID" {
817-
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
711+
if license := reqBody["license_template"]; license != "lgpl-3.0" {
712+
t.Errorf("expected %q, got %q", "lgpl-3.0", license)
818713
}
819714
}

0 commit comments

Comments
 (0)