Skip to content

Commit

Permalink
support refs with or without 'refs/' prefix
Browse files Browse the repository at this point in the history 
The git ref methods were mostly written to not expect the 'refs/' prefix
on ref names, even though it is included the ref.Ref value returned by
GitHub.  This resulted in ref values returned from some methods that
couldn't be passed to other methods.  In reality, ref names passed to
these methods should normally include the prefix, but this change
supports either form for backwards compatibility.

Fixes #133
  • Loading branch information
@willnorris
willnorris committed Sep 4, 2014
1 parent d8cbc48 commit 212ef69
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
9 changes: 7 additions & 2 deletions github/git_refs.go
View file
Expand Up @@ -7,6 +7,7 @@ package github

import (
"fmt"
"strings"
)

// Reference represents a GitHub reference.
Expand Down Expand Up @@ -47,6 +48,7 @@ type updateRefRequest struct {
//
// GitHub API docs: http://developer.github.com/v3/git/refs/#get-a-reference
func (s *GitService) GetRef(owner string, repo string, ref string) (*Reference, *Response, error) {
ref = strings.TrimPrefix(ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, ref)
req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
Expand Down Expand Up @@ -105,7 +107,8 @@ func (s *GitService) ListRefs(owner, repo string, opt *ReferenceListOptions) ([]
func (s *GitService) CreateRef(owner string, repo string, ref *Reference) (*Reference, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/git/refs", owner, repo)
req, err := s.client.NewRequest("POST", u, &createRefRequest{
Ref: String("refs/" + *ref.Ref),
// back-compat with previous behavior that didn't require 'refs/' prefix
Ref: String("refs/" + strings.TrimPrefix(*ref.Ref, "refs/")),
SHA: ref.Object.SHA,
})
if err != nil {
Expand All @@ -125,7 +128,8 @@ func (s *GitService) CreateRef(owner string, repo string, ref *Reference) (*Refe
//
// GitHub API docs: http://developer.github.com/v3/git/refs/#update-a-reference
func (s *GitService) UpdateRef(owner string, repo string, ref *Reference, force bool) (*Reference, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, *ref.Ref)
refPath := strings.TrimPrefix(*ref.Ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refPath)
req, err := s.client.NewRequest("PATCH", u, &updateRefRequest{
SHA: ref.Object.SHA,
Force: &force,
Expand All @@ -147,6 +151,7 @@ func (s *GitService) UpdateRef(owner string, repo string, ref *Reference, force
//
// GitHub API docs: http://developer.github.com/v3/git/refs/#delete-a-reference
func (s *GitService) DeleteRef(owner string, repo string, ref string) (*Response, error) {
ref = strings.TrimPrefix(ref, "refs/")
u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, ref)
req, err := s.client.NewRequest("DELETE", u, nil)
if err != nil {
Expand Down
38 changes: 34 additions & 4 deletions github/git_refs_test.go
View file
Expand Up @@ -31,7 +31,7 @@ func TestGitService_GetRef(t *testing.T) {
}`)
})

ref, _, err := client.Git.GetRef("o", "r", "heads/b")
ref, _, err := client.Git.GetRef("o", "r", "refs/heads/b")
if err != nil {
t.Errorf("Git.GetRef returned error: %v", err)
}
Expand All @@ -48,6 +48,11 @@ func TestGitService_GetRef(t *testing.T) {
if !reflect.DeepEqual(ref, want) {
t.Errorf("Git.GetRef returned %+v, want %+v", ref, want)
}

// without 'refs/' prefix
if _, _, err := client.Git.GetRef("o", "r", "heads/b"); err != nil {
t.Errorf("Git.GetRef returned error: %v", err)
}
}

func TestGitService_ListRefs(t *testing.T) {
Expand Down Expand Up @@ -161,7 +166,7 @@ func TestGitService_CreateRef(t *testing.T) {
})

ref, _, err := client.Git.CreateRef("o", "r", &Reference{
Ref: String("heads/b"),
Ref: String("refs/heads/b"),
Object: &GitObject{
SHA: String("aa218f56b14c9653891f9e74264a383fa43fefbd"),
},
Expand All @@ -182,6 +187,17 @@ func TestGitService_CreateRef(t *testing.T) {
if !reflect.DeepEqual(ref, want) {
t.Errorf("Git.CreateRef returned %+v, want %+v", ref, want)
}

// without 'refs/' prefix
_, _, err = client.Git.CreateRef("o", "r", &Reference{
Ref: String("heads/b"),
Object: &GitObject{
SHA: String("aa218f56b14c9653891f9e74264a383fa43fefbd"),
},
})
if err != nil {
t.Errorf("Git.CreateRef returned error: %v", err)
}
}

func TestGitService_UpdateRef(t *testing.T) {
Expand Down Expand Up @@ -214,7 +230,7 @@ func TestGitService_UpdateRef(t *testing.T) {
})

ref, _, err := client.Git.UpdateRef("o", "r", &Reference{
Ref: String("heads/b"),
Ref: String("refs/heads/b"),
Object: &GitObject{SHA: String("aa218f56b14c9653891f9e74264a383fa43fefbd")},
}, true)
if err != nil {
Expand All @@ -233,6 +249,15 @@ func TestGitService_UpdateRef(t *testing.T) {
if !reflect.DeepEqual(ref, want) {
t.Errorf("Git.UpdateRef returned %+v, want %+v", ref, want)
}

// without 'refs/' prefix
_, _, err = client.Git.UpdateRef("o", "r", &Reference{
Ref: String("heads/b"),
Object: &GitObject{SHA: String("aa218f56b14c9653891f9e74264a383fa43fefbd")},
}, true)
if err != nil {
t.Errorf("Git.UpdateRef returned error: %v", err)
}
}

func TestGitService_DeleteRef(t *testing.T) {
Expand All @@ -243,8 +268,13 @@ func TestGitService_DeleteRef(t *testing.T) {
testMethod(t, r, "DELETE")
})

_, err := client.Git.DeleteRef("o", "r", "heads/b")
_, err := client.Git.DeleteRef("o", "r", "refs/heads/b")
if err != nil {
t.Errorf("Git.DeleteRef returned error: %v", err)
}

// without 'refs/' prefix
if _, err := client.Git.DeleteRef("o", "r", "heads/b"); err != nil {
t.Errorf("Git.DeleteRef returned error: %v", err)
}
}

0 comments on commit 212ef69

Please sign in to comment.