From 212ef69dedaae61a77415911e32da7d1d6e3f2ec Mon Sep 17 00:00:00 2001 From: Will Norris Date: Thu, 4 Sep 2014 08:04:46 -0700 Subject: [PATCH] support refs with or without 'refs/' prefix 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 --- github/git_refs.go | 9 +++++++-- github/git_refs_test.go | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/github/git_refs.go b/github/git_refs.go index a0b4ce4..3d2f6c8 100644 --- a/github/git_refs.go +++ b/github/git_refs.go @@ -7,6 +7,7 @@ package github import ( "fmt" + "strings" ) // Reference represents a GitHub reference. @@ -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 { @@ -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 { @@ -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, @@ -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 { diff --git a/github/git_refs_test.go b/github/git_refs_test.go index e75092e..e66bf54 100644 --- a/github/git_refs_test.go +++ b/github/git_refs_test.go @@ -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) } @@ -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) { @@ -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"), }, @@ -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) { @@ -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 { @@ -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) { @@ -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) + } }