Repository: google/go-github
Pull request #1348 (2019-12-07 15:40:44)

Add context nil check on client Doer
I am adding a nil check for the provided context and return an error instead of panic.
@gmlewis raised a valid point that this is a user error but probably is better to return and error than panic.

Related to #1260 


Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e2a2b282..bd7db848 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -64,6 +64,9 @@ are more sensitive, emailed to <opensource@google.com>.
      messages are able to be formatted properly by various git tools.
 
   1. Finally, push the commits to your fork and submit a [pull request][].
+     **NOTE:** Please do not use force-push on PRs in this repo, as it makes
+     it more difficult for reviewers to see what has changed since the last
+     code review.
 
 [forking]: https://help.github.com/articles/fork-a-repo
 [golint]: https://github.com/golang/lint
diff --git a/github/github-accessors.go b/github/github-accessors.go
index b26ee6b7..1454e221 100644
--- a/github/github-accessors.go
+++ b/github/github-accessors.go
@@ -6548,6 +6548,30 @@ func (o *Organization) GetMembersAllowedRepositoryCreationType() string {
 	return *o.MembersAllowedRepositoryCreationType
 }
 
+// GetMembersCanCreateInternalRepos returns the MembersCanCreateInternalRepos field if it's non-nil, zero value otherwise.
+func (o *Organization) GetMembersCanCreateInternalRepos() bool {
+	if o == nil || o.MembersCanCreateInternalRepos == nil {
+		return false
+	}
+	return *o.MembersCanCreateInternalRepos
+}
+
+// GetMembersCanCreatePrivateRepos returns the MembersCanCreatePrivateRepos field if it's non-nil, zero value otherwise.
+func (o *Organization) GetMembersCanCreatePrivateRepos() bool {
+	if o == nil || o.MembersCanCreatePrivateRepos == nil {
+		return false
+	}
+	return *o.MembersCanCreatePrivateRepos
+}
+
+// GetMembersCanCreatePublicRepos returns the MembersCanCreatePublicRepos field if it's non-nil, zero value otherwise.
+func (o *Organization) GetMembersCanCreatePublicRepos() bool {
+	if o == nil || o.MembersCanCreatePublicRepos == nil {
+		return false
+	}
+	return *o.MembersCanCreatePublicRepos
+}
+
 // GetMembersCanCreateRepos returns the MembersCanCreateRepos field if it's non-nil, zero value otherwise.
 func (o *Organization) GetMembersCanCreateRepos() bool {
 	if o == nil || o.MembersCanCreateRepos == nil {
diff --git a/github/github.go b/github/github.go
index 3f4edd4a..1084412c 100644
--- a/github/github.go
+++ b/github/github.go
@@ -498,9 +498,12 @@ func parseRate(r *http.Response) Rate {
 // first decode it. If rate limit is exceeded and reset time is in the future,
 // Do returns *RateLimitError immediately without making a network API call.
 //
-// The provided ctx must be non-nil. If it is canceled or times out,
+// The provided ctx must be non-nil, if it is nil an error is returned. If it is canceled or times out,
 // ctx.Err() will be returned.
 func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*Response, error) {
+	if ctx == nil {
+		return nil, errors.New("context must be non-nil")
+	}
 	req = withContext(ctx, req)
 
 	rateLimitCategory := category(req.URL.Path)
diff --git a/github/orgs.go b/github/orgs.go
index 5012c6c9..bae67dad 100644
--- a/github/orgs.go
+++ b/github/orgs.go
@@ -56,8 +56,17 @@ type Organization struct {
 	// MembersCanCreateRepos default value is true and is only used in Organizations.Edit.
 	MembersCanCreateRepos *bool `json:"members_can_create_repositories,omitempty"`
 
+	// https://developer.github.com/changes/2019-12-03-internal-visibility-changes/#rest-v3-api
+	MembersCanCreatePublicRepos   *bool `json:"members_can_create_public_repositories,omitempty"`
+	MembersCanCreatePrivateRepos  *bool `json:"members_can_create_private_repositories,omitempty"`
+	MembersCanCreateInternalRepos *bool `json:"members_can_create_internal_repositories,omitempty"`
+
 	// MembersAllowedRepositoryCreationType denotes if organization members can create repositories
 	// and the type of repositories they can create. Possible values are: "all", "private", or "none".
+	//
+	// Deprecated: Use MembersCanCreatePublicRepos, MembersCanCreatePrivateRepos, MembersCanCreateInternalRepos
+	// instead. The new fields overrides the existing MembersAllowedRepositoryCreationType during 'edit'
+	// operation and does not consider 'internal' repositories during 'get' operation
 	MembersAllowedRepositoryCreationType *string `json:"members_allowed_repository_creation_type,omitempty"`
 
 	// API URLs
@@ -213,6 +222,9 @@ func (s *OrganizationsService) Edit(ctx context.Context, name string, org *Organ
 		return nil, nil, err
 	}
 
+	// TODO: remove custom Accept header when this API fully launches.
+	req.Header.Set("Accept", mediaTypeMemberAllowedRepoCreationTypePreview)
+
 	o := new(Organization)
 	resp, err := s.client.Do(ctx, req, o)
 	if err != nil {


Test patch:
diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go
index 04cd771c..2216c8c4 100644
--- a/github/github-stringify_test.go
+++ b/github/github-stringify_test.go
@@ -852,6 +852,9 @@ func TestOrganization_String(t *testing.T) {
 		DefaultRepoPermission:                String(""),
 		DefaultRepoSettings:                  String(""),
 		MembersCanCreateRepos:                Bool(false),
+		MembersCanCreatePublicRepos:          Bool(false),
+		MembersCanCreatePrivateRepos:         Bool(false),
+		MembersCanCreateInternalRepos:        Bool(false),
 		MembersAllowedRepositoryCreationType: String(""),
 		URL:                                  String(""),
 		EventsURL:                            String(""),
@@ -861,7 +864,7 @@ func TestOrganization_String(t *testing.T) {
 		PublicMembersURL:                     String(""),
 		ReposURL:                             String(""),
 	}
-	want := `github.Organization{Login:"", ID:0, NodeID:"", AvatarURL:"", HTMLURL:"", Name:"", Company:"", Blog:"", Location:"", Email:"", Description:"", PublicRepos:0, PublicGists:0, Followers:0, Following:0, TotalPrivateRepos:0, OwnedPrivateRepos:0, PrivateGists:0, DiskUsage:0, Collaborators:0, BillingEmail:"", Type:"", Plan:github.Plan{}, TwoFactorRequirementEnabled:false, DefaultRepoPermission:"", DefaultRepoSettings:"", MembersCanCreateRepos:false, MembersAllowedRepositoryCreationType:"", URL:"", EventsURL:"", HooksURL:"", IssuesURL:"", MembersURL:"", PublicMembersURL:"", ReposURL:""}`
+	want := `github.Organization{Login:"", ID:0, NodeID:"", AvatarURL:"", HTMLURL:"", Name:"", Company:"", Blog:"", Location:"", Email:"", Description:"", PublicRepos:0, PublicGists:0, Followers:0, Following:0, TotalPrivateRepos:0, OwnedPrivateRepos:0, PrivateGists:0, DiskUsage:0, Collaborators:0, BillingEmail:"", Type:"", Plan:github.Plan{}, TwoFactorRequirementEnabled:false, DefaultRepoPermission:"", DefaultRepoSettings:"", MembersCanCreateRepos:false, MembersCanCreatePublicRepos:false, MembersCanCreatePrivateRepos:false, MembersCanCreateInternalRepos:false, MembersAllowedRepositoryCreationType:"", URL:"", EventsURL:"", HooksURL:"", IssuesURL:"", MembersURL:"", PublicMembersURL:"", ReposURL:""}`
 	if got := v.String(); got != want {
 		t.Errorf("Organization.String = %v, want %v", got, want)
 	}
diff --git a/github/github_test.go b/github/github_test.go
index 759d76c5..1aa3ef9b 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -8,6 +8,7 @@ package github
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -477,6 +478,18 @@ func TestDo(t *testing.T) {
 	}
 }
 
+func TestDo_nilContext(t *testing.T) {
+	client, _, _, teardown := setup()
+	defer teardown()
+
+	req, _ := client.NewRequest("GET", ".", nil)
+	_, err := client.Do(nil, req, nil)
+
+	if !reflect.DeepEqual(err, errors.New("context must be non-nil")) {
+		t.Errorf("Expected context must be non-nil error")
+	}
+}
+
 func TestDo_httpError(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
diff --git a/github/orgs_test.go b/github/orgs_test.go
index c39f6097..cf69453c 100644
--- a/github/orgs_test.go
+++ b/github/orgs_test.go
@@ -27,6 +27,9 @@ func TestOrganization_marshal(t *testing.T) {
 		Description:                          String("GitHub, the company."),
 		DefaultRepoPermission:                String("read"),
 		MembersCanCreateRepos:                Bool(true),
+		MembersCanCreateInternalRepos:        Bool(true),
+		MembersCanCreatePrivateRepos:         Bool(true),
+		MembersCanCreatePublicRepos:          Bool(false),
 		MembersAllowedRepositoryCreationType: String("all"),
 	}
 	want := `
@@ -40,6 +43,9 @@ func TestOrganization_marshal(t *testing.T) {
 			"description": "GitHub, the company.",
 			"default_repository_permission": "read",
 			"members_can_create_repositories": true,
+			"members_can_create_public_repositories": false,
+			"members_can_create_private_repositories": true,
+			"members_can_create_internal_repositories": true,
 			"members_allowed_repository_creation_type": "all"
 		}
 	`
@@ -178,6 +184,7 @@ func TestOrganizationsService_Edit(t *testing.T) {
 		v := new(Organization)
 		json.NewDecoder(r.Body).Decode(v)
 
+		testHeader(t, r, "Accept", mediaTypeMemberAllowedRepoCreationTypePreview)
 		testMethod(t, r, "PATCH")
 		if !reflect.DeepEqual(v, input) {
 			t.Errorf("Request body = %+v, want %+v", v, input)