Updates for breaking changes to branch protection API

Per
<https://developer.github.com/changes/2016-06-27-protected-branches-api-update/>,
breaking changes were made to the protected branches API during the preview
period.

* Removes Branch.Protection
* Removes EditBranch, protection is now modified using separate API routes
* Adds GetBranchProtection
* Adds UpdateBranchProtection
* Adds RemoveBranchProtection

`omitempty` is intentionally left off fields in `ProtectionRequest` because
passing `nil` (JSON `null`) has semantic meaning (described in
<https://developer.github.com/v3/repos/branches/#update-branch-protection>)

Fixes #387.
Closes #476.

Change-Id: I674e1d3a486e80ce241e2fd8d14f7547c50697d4

github/repos.go

@@ -501,29 +501,54 @@ func (s *RepositoriesService) ListTags(owner string, repo string, opt *ListOptio
 
 // Branch represents a repository branch
 type Branch struct {
-	Name       *string     `json:"name,omitempty"`
-	Commit     *Commit     `json:"commit,omitempty"`
-	Protection *Protection `json:"protection,omitempty"`
+	Name      *string `json:"name,omitempty"`
+	Commit    *Commit `json:"commit,omitempty"`
+	Protected *bool   `json:"protected,omitempty"`
 }
 
-// Protection represents a repository branch's protection
+// Protection represents a repository branch's protection.
 type Protection struct {
-	Enabled              *bool                 `json:"enabled,omitempty"`
-	RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks,omitempty"`
+	RequiredStatusChecks *RequiredStatusChecks `json:"required_status_checks"`
+	Restrictions         *BranchRestrictions   `json:"restrictions"`
 }
 
-// RequiredStatusChecks represents the protection status of a individual branch
+// ProtectionRequest represents a request to create/edit a branch's protection.
+type ProtectionRequest struct {
+	RequiredStatusChecks *RequiredStatusChecks      `json:"required_status_checks"`
+	Restrictions         *BranchRestrictionsRequest `json:"restrictions"`
+}
+
+// RequiredStatusChecks represents the protection status of a individual branch.
 type RequiredStatusChecks struct {
-	// Who required status checks apply to.
-	// Possible values are:
-	//     off
-	//     non_admins
-	//     everyone
-	EnforcementLevel *string `json:"enforcement_level,omitempty"`
-	// The list of status checks which are required
+	// Enforce required status checks for repository administrators.
+	IncludeAdmins *bool `json:"include_admins,omitempty"`
+	// Require branches to be up to date before merging.
+	Strict *bool `json:"strict,omitempty"`
+	// The list of status checks to require in order to merge into this
+	// branch.
 	Contexts *[]string `json:"contexts,omitempty"`
 }
 
+// BranchRestrictions represents the restriction that only certain users or
+// teams may push to a branch.
+type BranchRestrictions struct {
+	// The list of user logins with push access.
+	Users []*User `json:"users,omitempty"`
+	// The list of team slugs with push access.
+	Teams []*Team `json:"teams,omitempty"`
+}
+
+// BranchRestrictionsRequest represents the request to create/edit the
+// restriction that only certain users or teams may push to a branch. It is
+// separate from BranchRestrictions above because the request structure is
+// different from the response structure.
+type BranchRestrictionsRequest struct {
+	// The list of user logins with push access.
+	Users *[]string `json:"users,omitempty"`
+	// The list of team slugs with push access.
+	Teams *[]string `json:"teams,omitempty"`
+}
+
 // ListBranches lists branches for the specified repository.
 //
 // GitHub API docs: http://developer.github.com/v3/repos/#list-branches
@@ -539,6 +564,7 @@ func (s *RepositoriesService) ListBranches(owner string, repo string, opt *ListO
 		return nil, nil, err
 	}
 
+	// TODO: remove custom Accept header when this API fully launches
 	req.Header.Set("Accept", mediaTypeProtectedBranchesPreview)
 
 	branches := new([]*Branch)
@@ -560,6 +586,7 @@ func (s *RepositoriesService) GetBranch(owner, repo, branch string) (*Branch, *R
 		return nil, nil, err
 	}
 
+	// TODO: remove custom Accept header when this API fully launches
 	req.Header.Set("Accept", mediaTypeProtectedBranchesPreview)
 
 	b := new(Branch)
@@ -571,25 +598,64 @@ func (s *RepositoriesService) GetBranch(owner, repo, branch string) (*Branch, *R
 	return b, resp, err
 }
 
-// EditBranch edits the branch (currently only Branch Protection)
+// GetBranchProtection gets the protection of a given branch.
 //
-// GitHub API docs: https://developer.github.com/v3/repos/#enabling-and-disabling-branch-protection
-func (s *RepositoriesService) EditBranch(owner, repo, branchName string, branch *Branch) (*Branch, *Response, error) {
-	u := fmt.Sprintf("repos/%v/%v/branches/%v", owner, repo, branchName)
-	req, err := s.client.NewRequest("PATCH", u, branch)
+// GitHub API docs: https://developer.github.com/v3/repos/branches/#get-branch-protection
+func (s *RepositoriesService) GetBranchProtection(owner, repo, branch string) (*Protection, *Response, error) {
+	u := fmt.Sprintf("repos/%v/%v/branches/%v/protection", owner, repo, branch)
+	req, err := s.client.NewRequest("GET", u, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
+	// TODO: remove custom Accept header when this API fully launches
 	req.Header.Set("Accept", mediaTypeProtectedBranchesPreview)
 
-	b := new(Branch)
-	resp, err := s.client.Do(req, b)
+	p := new(Protection)
+	resp, err := s.client.Do(req, p)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return b, resp, err
+	return p, resp, err
+}
+
+// UpdateBranchProtection updates the protection of a given branch.
+//
+// GitHub API docs: https://developer.github.com/v3/repos/branches/#update-branch-protection
+func (s *RepositoriesService) UpdateBranchProtection(owner, repo, branch string, preq *ProtectionRequest) (*Protection, *Response, error) {
+	u := fmt.Sprintf("repos/%v/%v/branches/%v/protection", owner, repo, branch)
+	req, err := s.client.NewRequest("PUT", u, preq)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// TODO: remove custom Accept header when this API fully launches
+	req.Header.Set("Accept", mediaTypeProtectedBranchesPreview)
+
+	p := new(Protection)
+	resp, err := s.client.Do(req, p)
+	if err != nil {
+		return nil, resp, err
+	}
+
+	return p, resp, err
+}
+
+// RemoveBranchProtection removes the protection of a given branch.
+//
+// GitHub API docs: https://developer.github.com/v3/repos/branches/#remove-branch-protection
+func (s *RepositoriesService) RemoveBranchProtection(owner, repo, branch string) (*Response, error) {
+	u := fmt.Sprintf("repos/%v/%v/branches/%v/protection", owner, repo, branch)
+	req, err := s.client.NewRequest("DELETE", u, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO: remove custom Accept header when this API fully launches
+	req.Header.Set("Accept", mediaTypeProtectedBranchesPreview)
+
+	return s.client.Do(req, nil)
 }
 
 // License gets the contents of a repository's license if one is detected.

github/repos_test.go

@@ -447,7 +447,7 @@ func TestRepositoriesService_GetBranch(t *testing.T) {
 	mux.HandleFunc("/repos/o/r/branches/b", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview)
-		fmt.Fprint(w, `{"name":"n", "commit":{"sha":"s"}, "protection": {"enabled": true, "required_status_checks": {"enforcement_level": "everyone","contexts": []}}}`)
+		fmt.Fprint(w, `{"name":"n", "commit":{"sha":"s"}, "protected":true}`)
 	})
 
 	branch, _, err := client.Repositories.GetBranch("o", "r", "b")
@@ -456,15 +456,9 @@ func TestRepositoriesService_GetBranch(t *testing.T) {
 	}
 
 	want := &Branch{
-		Name:   String("n"),
-		Commit: &Commit{SHA: String("s")},
-		Protection: &Protection{
-			Enabled: Bool(true),
-			RequiredStatusChecks: &RequiredStatusChecks{
-				EnforcementLevel: String("everyone"),
-				Contexts:         &[]string{},
-			},
-		},
+		Name:      String("n"),
+		Commit:    &Commit{SHA: String("s")},
+		Protected: Bool(true),
 	}
 
 	if !reflect.DeepEqual(branch, want) {
@@ -472,39 +466,110 @@ func TestRepositoriesService_GetBranch(t *testing.T) {
 	}
 }
 
-func TestRepositoriesService_EditBranch(t *testing.T) {
+func TestRepositoriesService_GetBranchProtection(t *testing.T) {
 	setup()
 	defer teardown()
 
-	input := &Branch{
-		Protection: &Protection{
-			Enabled: Bool(true),
-			RequiredStatusChecks: &RequiredStatusChecks{
-				EnforcementLevel: String("everyone"),
-				Contexts:         &[]string{"continous-integration"},
+	mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) {
+		v := new(ProtectionRequest)
+		json.NewDecoder(r.Body).Decode(v)
+
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview)
+		fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`)
+	})
+
+	protection, _, err := client.Repositories.GetBranchProtection("o", "r", "b")
+	if err != nil {
+		t.Errorf("Repositories.GetBranchProtection returned error: %v", err)
+	}
+
+	want := &Protection{
+		RequiredStatusChecks: &RequiredStatusChecks{
+			IncludeAdmins: Bool(true),
+			Strict:        Bool(true),
+			Contexts:      &[]string{"continuous-integration"},
+		},
+		Restrictions: &BranchRestrictions{
+			Users: []*User{
+				{Login: String("u"), ID: Int(1)},
+			},
+			Teams: []*Team{
+				{Slug: String("t"), ID: Int(2)},
 			},
 		},
 	}
+	if !reflect.DeepEqual(protection, want) {
+		t.Errorf("Repositories.GetBranchProtection returned %+v, want %+v", protection, want)
+	}
+}
 
-	mux.HandleFunc("/repos/o/r/branches/b", func(w http.ResponseWriter, r *http.Request) {
-		v := new(Branch)
+func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
+	setup()
+	defer teardown()
+
+	input := &ProtectionRequest{
+		RequiredStatusChecks: &RequiredStatusChecks{
+			IncludeAdmins: Bool(true),
+			Strict:        Bool(true),
+			Contexts:      &[]string{"continuous-integration"},
+		},
+		Restrictions: &BranchRestrictionsRequest{
+			Users: &[]string{"u"},
+			Teams: &[]string{"t"},
+		},
+	}
+
+	mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) {
+		v := new(ProtectionRequest)
 		json.NewDecoder(r.Body).Decode(v)
 
-		testMethod(t, r, "PATCH")
+		testMethod(t, r, "PUT")
 		if !reflect.DeepEqual(v, input) {
 			t.Errorf("Request body = %+v, want %+v", v, input)
 		}
 		testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview)
-		fmt.Fprint(w, `{"protection": {"enabled": true, "required_status_checks": {"enforcement_level": "everyone", "contexts": ["continous-integration"]}}}`)
+		fmt.Fprintf(w, `{"required_status_checks":{"include_admins":true,"strict":true,"contexts":["continuous-integration"]},"restrictions":{"users":[{"id":1,"login":"u"}],"teams":[{"id":2,"slug":"t"}]}}`)
 	})
 
-	branch, _, err := client.Repositories.EditBranch("o", "r", "b", input)
+	protection, _, err := client.Repositories.UpdateBranchProtection("o", "r", "b", input)
 	if err != nil {
-		t.Errorf("Repositories.EditBranch returned error: %v", err)
+		t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err)
+	}
+
+	want := &Protection{
+		RequiredStatusChecks: &RequiredStatusChecks{
+			IncludeAdmins: Bool(true),
+			Strict:        Bool(true),
+			Contexts:      &[]string{"continuous-integration"},
+		},
+		Restrictions: &BranchRestrictions{
+			Users: []*User{
+				{Login: String("u"), ID: Int(1)},
+			},
+			Teams: []*Team{
+				{Slug: String("t"), ID: Int(2)},
+			},
+		},
 	}
+	if !reflect.DeepEqual(protection, want) {
+		t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want)
+	}
+}
+
+func TestRepositoriesService_RemoveBranchProtection(t *testing.T) {
+	setup()
+	defer teardown()
 
-	if !reflect.DeepEqual(branch, input) {
-		t.Errorf("Repositories.EditBranch returned %+v, want %+v", branch, input)
+	mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "DELETE")
+		testHeader(t, r, "Accept", mediaTypeProtectedBranchesPreview)
+		w.WriteHeader(http.StatusNoContent)
+	})
+
+	_, err := client.Repositories.RemoveBranchProtection("o", "r", "b")
+	if err != nil {
+		t.Errorf("Repositories.RemoveBranchProtection returned error: %v", err)
 	}
 }