From 3f0a0a324070313a3bbf88102b7dccd5599407dc Mon Sep 17 00:00:00 2001 From: Marcelo Jorge Vieira Date: Wed, 23 Jul 2014 17:11:51 -0300 Subject: [PATCH 01/10] Repository: Improved GetForEachRef Removed unnecessary replace --- repository/repository.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/repository/repository.go b/repository/repository.go index 6cf6b6a..348dd9e 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -362,7 +362,7 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st if err != nil || !repoExists { return nil, fmt.Errorf("Error when trying to obtain the branches of repository %s (Repository does not exist).", repo) } - cmd := exec.Command(gitPath, "for-each-ref", "--sort=-committerdate", "--format", "%(objectname)%09%(refname)%09%(committername)%09%(committeremail)%09%(committerdate)%09%(authorname)%09%(authoremail)%09%(authordate)%09%(contents:subject)", pattern) + cmd := exec.Command(gitPath, "for-each-ref", "--sort=-committerdate", "--format", "%(objectname)%09%(refname:short)%09%(committername)%09%(committeremail)%09%(committerdate)%09%(authorname)%09%(authoremail)%09%(authordate)%09%(contents:subject)", pattern) cmd.Dir = cwd out, err := cmd.Output() if err != nil { @@ -379,7 +379,7 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st fields := strings.Split(line, "\t") if len(fields) > 4 { // let there be commits with empty subject ref = fields[0] - name = strings.Replace(fields[1], pattern, "", 1) + name = fields[1] commiterName = fields[2] commiterEmail = fields[3] commiterDate = fields[4] From fe183f195f43b01d40406e9d778ed5948db25e1e Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Wed, 23 Jul 2014 17:22:29 -0300 Subject: [PATCH 02/10] Fix error message, make it generic --- repository/repository.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/repository/repository.go b/repository/repository.go index 348dd9e..f3a006d 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -355,18 +355,18 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st var ref, name, commiterName, commiterEmail, commiterDate, authorName, authorEmail, authorDate, subject string gitPath, err := exec.LookPath("git") if err != nil { - return nil, fmt.Errorf("Error when trying to obtain the branches of repository %s (%s).", repo, err) + return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (%s).", repo, err) } cwd := barePath(repo) repoExists, err := exists(cwd) if err != nil || !repoExists { - return nil, fmt.Errorf("Error when trying to obtain the branches of repository %s (Repository does not exist).", repo) + return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (Repository does not exist).", repo) } cmd := exec.Command(gitPath, "for-each-ref", "--sort=-committerdate", "--format", "%(objectname)%09%(refname:short)%09%(committername)%09%(committeremail)%09%(committerdate)%09%(authorname)%09%(authoremail)%09%(authordate)%09%(contents:subject)", pattern) cmd.Dir = cwd out, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("Error when trying to obtain the branches of repository %s (%s).", repo, err) + return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (%s).", repo, err) } lines := strings.Split(strings.TrimSpace(string(out)), "\n") objectCount := len(lines) @@ -388,7 +388,7 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st authorDate = fields[7] subject = strings.Join(fields[8:], "\t") // let there be subjects with \t } else { - return nil, fmt.Errorf("Error when trying to obtain the branches of repository %s (Invalid git for-each-ref output [%s]).", repo, out) + return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (Invalid git for-each-ref output [%s]).", repo, out) } object := make(map[string]string) object["ref"] = ref From 9ab48fc8a58edeed2782687a2dce279e547fba6e Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Wed, 23 Jul 2014 18:46:41 -0300 Subject: [PATCH 03/10] Improve mock/tests for GetBranch (re #123) --- repository/mocks.go | 27 +-------------------------- repository/repository_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 30 deletions(-) diff --git a/repository/mocks.go b/repository/mocks.go index 3bd1b66..1cc0852 100644 --- a/repository/mocks.go +++ b/repository/mocks.go @@ -103,7 +103,7 @@ func CreateTestRepository(tmp_path string, repo string, file string, content str return cleanup, err } -func CreateBranchesOnTestRepository(tmp_path string, repo string, file string, content string, branches ...string) error { +func CreateBranchesOnTestRepository(tmp_path string, repo string, branches ...string) error { testPath := path.Join(tmp_path, repo+".git") gitPath, err := exec.LookPath("git") if err != nil { @@ -116,37 +116,12 @@ func CreateBranchesOnTestRepository(tmp_path string, repo string, file string, c return err } for _, branch := range branches { - fp, err := os.OpenFile(path.Join(testPath, file), os.O_APPEND|os.O_WRONLY, 0644) - if err != nil { - return err - } - defer fp.Close() - _, err = fp.WriteString("such string") - if err != nil { - return err - } cmd = exec.Command(gitPath, "checkout", "-b", branch) cmd.Dir = testPath err = cmd.Run() if err != nil { return err } - cmd = exec.Command(gitPath, "add", ".") - cmd.Dir = testPath - err = cmd.Run() - if err != nil { - return err - } - if len(content) > 0 { - cmd = exec.Command(gitPath, "commit", "-m", content+" on "+branch) - } else { - cmd = exec.Command(gitPath, "commit", "-m", "", "--allow-empty-message") - } - cmd.Dir = testPath - err = cmd.Run() - if err != nil { - return err - } } return err } diff --git a/repository/repository_test.go b/repository/repository_test.go index 90a0029..64af332 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -816,7 +816,7 @@ func (s *S) TestGetBranchIntegration(c *gocheck.C) { bare = oldBare }() c.Assert(errCreate, gocheck.IsNil) - errCreateBranches := CreateBranchesOnTestRepository(bare, repo, file, content, "doge_bites", "doge_barks") + errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_bites", "doge_barks") c.Assert(errCreateBranches, gocheck.IsNil) branches, err := GetBranch(repo) c.Assert(err, gocheck.IsNil) @@ -827,14 +827,14 @@ func (s *S) TestGetBranchIntegration(c *gocheck.C) { c.Assert(branches[0]["commiterEmail"], gocheck.Equals, "") c.Assert(branches[0]["authorName"], gocheck.Equals, "doge") c.Assert(branches[0]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[0]["subject"], gocheck.Equals, "will bark on doge_barks") + c.Assert(branches[0]["subject"], gocheck.Equals, "will bark") c.Assert(branches[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") c.Assert(branches[1]["name"], gocheck.Equals, "doge_bites") c.Assert(branches[1]["commiterName"], gocheck.Equals, "doge") c.Assert(branches[1]["commiterEmail"], gocheck.Equals, "") c.Assert(branches[1]["authorName"], gocheck.Equals, "doge") c.Assert(branches[1]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[1]["subject"], gocheck.Equals, "will bark on doge_bites") + c.Assert(branches[1]["subject"], gocheck.Equals, "will bark") c.Assert(branches[2]["ref"], gocheck.Matches, "[a-f0-9]{40}") c.Assert(branches[2]["name"], gocheck.Equals, "master") c.Assert(branches[2]["commiterName"], gocheck.Equals, "doge") @@ -856,7 +856,7 @@ func (s *S) TestGetBranchIntegrationEmptySubject(c *gocheck.C) { bare = oldBare }() c.Assert(errCreate, gocheck.IsNil) - errCreateBranches := CreateBranchesOnTestRepository(bare, repo, file, content, "doge_howls") + errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") c.Assert(errCreateBranches, gocheck.IsNil) branches, err := GetBranch(repo) c.Assert(err, gocheck.IsNil) From b8428996a0796d848bec64b8395d096f6799c969 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Wed, 23 Jul 2014 18:48:11 -0300 Subject: [PATCH 04/10] Add test for subject with tab (re #123) --- repository/repository_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/repository/repository_test.go b/repository/repository_test.go index 64af332..8f93ceb 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -876,3 +876,36 @@ func (s *S) TestGetBranchIntegrationEmptySubject(c *gocheck.C) { c.Assert(branches[1]["authorEmail"], gocheck.Equals, "") c.Assert(branches[1]["subject"], gocheck.Equals, "") } + +func (s *S) TestGetBranchIntegrationSubjectWithTab(c *gocheck.C) { + oldBare := bare + bare = "/tmp" + repo := "gandalf-test-repo" + file := "README" + content := "will\tbark" + cleanUp, errCreate := CreateTestRepository(bare, repo, file, content) + defer func() { + cleanUp() + bare = oldBare + }() + c.Assert(errCreate, gocheck.IsNil) + errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") + c.Assert(errCreateBranches, gocheck.IsNil) + branches, err := GetBranch(repo) + c.Assert(err, gocheck.IsNil) + c.Assert(len(branches), gocheck.Equals, 2) + c.Assert(branches[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(branches[0]["name"], gocheck.Equals, "doge_howls") + c.Assert(branches[0]["commiterName"], gocheck.Equals, "doge") + c.Assert(branches[0]["commiterEmail"], gocheck.Equals, "") + c.Assert(branches[0]["authorName"], gocheck.Equals, "doge") + c.Assert(branches[0]["authorEmail"], gocheck.Equals, "") + c.Assert(branches[0]["subject"], gocheck.Equals, "will\tbark") + c.Assert(branches[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(branches[1]["name"], gocheck.Equals, "master") + c.Assert(branches[1]["commiterName"], gocheck.Equals, "doge") + c.Assert(branches[1]["commiterEmail"], gocheck.Equals, "") + c.Assert(branches[1]["authorName"], gocheck.Equals, "doge") + c.Assert(branches[1]["authorEmail"], gocheck.Equals, "") + c.Assert(branches[1]["subject"], gocheck.Equals, "will\tbark") +} From 40ce1be119f3b173b5ccdf5d2e0cf18b2d3081a0 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Thu, 24 Jul 2014 10:20:39 -0300 Subject: [PATCH 05/10] Test GetForEachRef method instead of the one that uses it --- repository/repository_test.go | 76 +++++++++++++++++------------------ 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/repository/repository_test.go b/repository/repository_test.go index 8f93ceb..74d2ffa 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -844,7 +844,7 @@ func (s *S) TestGetBranchIntegration(c *gocheck.C) { c.Assert(branches[2]["subject"], gocheck.Equals, "will bark") } -func (s *S) TestGetBranchIntegrationEmptySubject(c *gocheck.C) { +func (s *S) TestGetForEachRefIntegrationEmptySubject(c *gocheck.C) { oldBare := bare bare = "/tmp" repo := "gandalf-test-repo" @@ -858,26 +858,26 @@ func (s *S) TestGetBranchIntegrationEmptySubject(c *gocheck.C) { c.Assert(errCreate, gocheck.IsNil) errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") c.Assert(errCreateBranches, gocheck.IsNil) - branches, err := GetBranch(repo) - c.Assert(err, gocheck.IsNil) - c.Assert(len(branches), gocheck.Equals, 2) - c.Assert(branches[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") - c.Assert(branches[0]["name"], gocheck.Equals, "doge_howls") - c.Assert(branches[0]["commiterName"], gocheck.Equals, "doge") - c.Assert(branches[0]["commiterEmail"], gocheck.Equals, "") - c.Assert(branches[0]["authorName"], gocheck.Equals, "doge") - c.Assert(branches[0]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[0]["subject"], gocheck.Equals, "") - c.Assert(branches[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") - c.Assert(branches[1]["name"], gocheck.Equals, "master") - c.Assert(branches[1]["commiterName"], gocheck.Equals, "doge") - c.Assert(branches[1]["commiterEmail"], gocheck.Equals, "") - c.Assert(branches[1]["authorName"], gocheck.Equals, "doge") - c.Assert(branches[1]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[1]["subject"], gocheck.Equals, "") -} - -func (s *S) TestGetBranchIntegrationSubjectWithTab(c *gocheck.C) { + refs, err := GetForEachRef(repo, "refs/") + c.Assert(err, gocheck.IsNil) + c.Assert(len(refs), gocheck.Equals, 2) + c.Assert(refs[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(refs[0]["name"], gocheck.Equals, "doge_howls") + c.Assert(refs[0]["commiterName"], gocheck.Equals, "doge") + c.Assert(refs[0]["commiterEmail"], gocheck.Equals, "") + c.Assert(refs[0]["authorName"], gocheck.Equals, "doge") + c.Assert(refs[0]["authorEmail"], gocheck.Equals, "") + c.Assert(refs[0]["subject"], gocheck.Equals, "") + c.Assert(refs[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(refs[1]["name"], gocheck.Equals, "master") + c.Assert(refs[1]["commiterName"], gocheck.Equals, "doge") + c.Assert(refs[1]["commiterEmail"], gocheck.Equals, "") + c.Assert(refs[1]["authorName"], gocheck.Equals, "doge") + c.Assert(refs[1]["authorEmail"], gocheck.Equals, "") + c.Assert(refs[1]["subject"], gocheck.Equals, "") +} + +func (s *S) TestGetForEachRefIntegrationSubjectWithTab(c *gocheck.C) { oldBare := bare bare = "/tmp" repo := "gandalf-test-repo" @@ -891,21 +891,21 @@ func (s *S) TestGetBranchIntegrationSubjectWithTab(c *gocheck.C) { c.Assert(errCreate, gocheck.IsNil) errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") c.Assert(errCreateBranches, gocheck.IsNil) - branches, err := GetBranch(repo) - c.Assert(err, gocheck.IsNil) - c.Assert(len(branches), gocheck.Equals, 2) - c.Assert(branches[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") - c.Assert(branches[0]["name"], gocheck.Equals, "doge_howls") - c.Assert(branches[0]["commiterName"], gocheck.Equals, "doge") - c.Assert(branches[0]["commiterEmail"], gocheck.Equals, "") - c.Assert(branches[0]["authorName"], gocheck.Equals, "doge") - c.Assert(branches[0]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[0]["subject"], gocheck.Equals, "will\tbark") - c.Assert(branches[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") - c.Assert(branches[1]["name"], gocheck.Equals, "master") - c.Assert(branches[1]["commiterName"], gocheck.Equals, "doge") - c.Assert(branches[1]["commiterEmail"], gocheck.Equals, "") - c.Assert(branches[1]["authorName"], gocheck.Equals, "doge") - c.Assert(branches[1]["authorEmail"], gocheck.Equals, "") - c.Assert(branches[1]["subject"], gocheck.Equals, "will\tbark") + refs, err := GetForEachRef(repo, "refs/") + c.Assert(err, gocheck.IsNil) + c.Assert(len(refs), gocheck.Equals, 2) + c.Assert(refs[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(refs[0]["name"], gocheck.Equals, "doge_howls") + c.Assert(refs[0]["commiterName"], gocheck.Equals, "doge") + c.Assert(refs[0]["commiterEmail"], gocheck.Equals, "") + c.Assert(refs[0]["authorName"], gocheck.Equals, "doge") + c.Assert(refs[0]["authorEmail"], gocheck.Equals, "") + c.Assert(refs[0]["subject"], gocheck.Equals, "will\tbark") + c.Assert(refs[1]["ref"], gocheck.Matches, "[a-f0-9]{40}") + c.Assert(refs[1]["name"], gocheck.Equals, "master") + c.Assert(refs[1]["commiterName"], gocheck.Equals, "doge") + c.Assert(refs[1]["commiterEmail"], gocheck.Equals, "") + c.Assert(refs[1]["authorName"], gocheck.Equals, "doge") + c.Assert(refs[1]["authorEmail"], gocheck.Equals, "") + c.Assert(refs[1]["subject"], gocheck.Equals, "will\tbark") } From 433787e0ab0834714bf7111aa421bef481486ce9 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Thu, 24 Jul 2014 11:14:01 -0300 Subject: [PATCH 06/10] Warily treat an empty or invalid pattern (re #120 and #123) --- repository/repository.go | 5 +++++ repository/repository_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/repository/repository.go b/repository/repository.go index f3a006d..0271523 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -369,6 +369,11 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (%s).", repo, err) } lines := strings.Split(strings.TrimSpace(string(out)), "\n") + // When the separator does not separate the string, Split returns an array + // with one element: the string itself. (golang.org/pkg/strings/#Split) + if len(lines) == 1 && len(lines[0]) == 0 { + return nil, nil + } objectCount := len(lines) objects := make([]map[string]string, objectCount) objectCount = 0 diff --git a/repository/repository_test.go b/repository/repository_test.go index 74d2ffa..59b006c 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -909,3 +909,37 @@ func (s *S) TestGetForEachRefIntegrationSubjectWithTab(c *gocheck.C) { c.Assert(refs[1]["authorEmail"], gocheck.Equals, "") c.Assert(refs[1]["subject"], gocheck.Equals, "will\tbark") } + +func (s *S) TestGetForEachRefIntegrationWhenPatternEmpty(c *gocheck.C) { + oldBare := bare + bare = "/tmp" + repo := "gandalf-test-repo" + file := "README" + content := "much WOW" + cleanUp, errCreate := CreateTestRepository(bare, repo, file, content) + defer func() { + cleanUp() + bare = oldBare + }() + c.Assert(errCreate, gocheck.IsNil) + refs, err := GetForEachRef("gandalf-test-repo", "") + c.Assert(refs, gocheck.IsNil) + c.Assert(err, gocheck.IsNil) +} + +func (s *S) TestGetForEachRefIntegrationWhenPatternInvalid(c *gocheck.C) { + oldBare := bare + bare = "/tmp" + repo := "gandalf-test-repo" + file := "README" + content := "much WOW" + cleanUp, errCreate := CreateTestRepository(bare, repo, file, content) + defer func() { + cleanUp() + bare = oldBare + }() + c.Assert(errCreate, gocheck.IsNil) + refs, err := GetForEachRef("gandalf-test-repo", "invalid_pattern") + c.Assert(refs, gocheck.IsNil) + c.Assert(err, gocheck.IsNil) +} From da67d88ed32d86df9a08cb68467d69897b60fb60 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Thu, 24 Jul 2014 16:32:15 -0300 Subject: [PATCH 07/10] Correctly accept an empty pattern (re #120 and #123) --- repository/repository.go | 11 +++++------ repository/repository_test.go | 6 ++++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/repository/repository.go b/repository/repository.go index 0271523..9446996 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -362,18 +362,17 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st if err != nil || !repoExists { return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (Repository does not exist).", repo) } - cmd := exec.Command(gitPath, "for-each-ref", "--sort=-committerdate", "--format", "%(objectname)%09%(refname:short)%09%(committername)%09%(committeremail)%09%(committerdate)%09%(authorname)%09%(authoremail)%09%(authordate)%09%(contents:subject)", pattern) + format := "%(objectname)%09%(refname:short)%09%(committername)%09%(committeremail)%09%(committerdate)%09%(authorname)%09%(authoremail)%09%(authordate)%09%(contents:subject)" + cmd := exec.Command(gitPath, "for-each-ref", "--sort=-committerdate", "--format", format) + if len(pattern) > 0 { + cmd.Args = append(cmd.Args, pattern) + } cmd.Dir = cwd out, err := cmd.Output() if err != nil { return nil, fmt.Errorf("Error when trying to obtain the refs of repository %s (%s).", repo, err) } lines := strings.Split(strings.TrimSpace(string(out)), "\n") - // When the separator does not separate the string, Split returns an array - // with one element: the string itself. (golang.org/pkg/strings/#Split) - if len(lines) == 1 && len(lines[0]) == 0 { - return nil, nil - } objectCount := len(lines) objects := make([]map[string]string, objectCount) objectCount = 0 diff --git a/repository/repository_test.go b/repository/repository_test.go index 59b006c..095c907 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -922,9 +922,11 @@ func (s *S) TestGetForEachRefIntegrationWhenPatternEmpty(c *gocheck.C) { bare = oldBare }() c.Assert(errCreate, gocheck.IsNil) + errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") + c.Assert(errCreateBranches, gocheck.IsNil) refs, err := GetForEachRef("gandalf-test-repo", "") - c.Assert(refs, gocheck.IsNil) c.Assert(err, gocheck.IsNil) + c.Assert(len(refs), gocheck.Equals, 2) } func (s *S) TestGetForEachRefIntegrationWhenPatternInvalid(c *gocheck.C) { @@ -940,6 +942,6 @@ func (s *S) TestGetForEachRefIntegrationWhenPatternInvalid(c *gocheck.C) { }() c.Assert(errCreate, gocheck.IsNil) refs, err := GetForEachRef("gandalf-test-repo", "invalid_pattern") - c.Assert(refs, gocheck.IsNil) c.Assert(err, gocheck.IsNil) + c.Assert(len(refs), gocheck.Equals, 1) } From f943fc7b1d0010ae826938f84ee2f2f15215d87f Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Thu, 24 Jul 2014 17:14:53 -0300 Subject: [PATCH 08/10] Improve GetForEachRef() tests (re #120 and #123) --- repository/repository_test.go | 61 +++++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/repository/repository_test.go b/repository/repository_test.go index 095c907..72ff84a 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -820,7 +820,7 @@ func (s *S) TestGetBranchIntegration(c *gocheck.C) { c.Assert(errCreateBranches, gocheck.IsNil) branches, err := GetBranch(repo) c.Assert(err, gocheck.IsNil) - c.Assert(len(branches), gocheck.Equals, 3) + c.Assert(branches, gocheck.HasLen, 3) c.Assert(branches[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") c.Assert(branches[0]["name"], gocheck.Equals, "doge_barks") c.Assert(branches[0]["commiterName"], gocheck.Equals, "doge") @@ -844,7 +844,7 @@ func (s *S) TestGetBranchIntegration(c *gocheck.C) { c.Assert(branches[2]["subject"], gocheck.Equals, "will bark") } -func (s *S) TestGetForEachRefIntegrationEmptySubject(c *gocheck.C) { +func (s *S) TestGetForEachRefIntegrationWithSubjectEmpty(c *gocheck.C) { oldBare := bare bare = "/tmp" repo := "gandalf-test-repo" @@ -860,7 +860,7 @@ func (s *S) TestGetForEachRefIntegrationEmptySubject(c *gocheck.C) { c.Assert(errCreateBranches, gocheck.IsNil) refs, err := GetForEachRef(repo, "refs/") c.Assert(err, gocheck.IsNil) - c.Assert(len(refs), gocheck.Equals, 2) + c.Assert(refs, gocheck.HasLen, 2) c.Assert(refs[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") c.Assert(refs[0]["name"], gocheck.Equals, "doge_howls") c.Assert(refs[0]["commiterName"], gocheck.Equals, "doge") @@ -877,7 +877,7 @@ func (s *S) TestGetForEachRefIntegrationEmptySubject(c *gocheck.C) { c.Assert(refs[1]["subject"], gocheck.Equals, "") } -func (s *S) TestGetForEachRefIntegrationSubjectWithTab(c *gocheck.C) { +func (s *S) TestGetForEachRefIntegrationWithSubjectTabbed(c *gocheck.C) { oldBare := bare bare = "/tmp" repo := "gandalf-test-repo" @@ -893,7 +893,7 @@ func (s *S) TestGetForEachRefIntegrationSubjectWithTab(c *gocheck.C) { c.Assert(errCreateBranches, gocheck.IsNil) refs, err := GetForEachRef(repo, "refs/") c.Assert(err, gocheck.IsNil) - c.Assert(len(refs), gocheck.Equals, 2) + c.Assert(refs, gocheck.HasLen, 2) c.Assert(refs[0]["ref"], gocheck.Matches, "[a-f0-9]{40}") c.Assert(refs[0]["name"], gocheck.Equals, "doge_howls") c.Assert(refs[0]["commiterName"], gocheck.Equals, "doge") @@ -911,6 +911,47 @@ func (s *S) TestGetForEachRefIntegrationSubjectWithTab(c *gocheck.C) { } func (s *S) TestGetForEachRefIntegrationWhenPatternEmpty(c *gocheck.C) { + oldBare := bare + bare = "/tmp" + repo := "gandalf-test-repo" + file := "README" + content := "much WOW" + cleanUp, errCreate := CreateTestRepository(bare, repo, file, content) + defer func() { + cleanUp() + bare = oldBare + }() + c.Assert(errCreate, gocheck.IsNil) + refs, err := GetForEachRef("gandalf-test-repo", "") + c.Assert(err, gocheck.IsNil) + c.Assert(refs, gocheck.HasLen, 1) + c.Assert(refs[0], gocheck.HasLen, 9) +} + +func (s *S) TestGetForEachRefIntegrationWhenPatternNonExistent(c *gocheck.C) { + oldBare := bare + bare = "/tmp" + repo := "gandalf-test-repo" + file := "README" + content := "much WOW" + cleanUp, errCreate := CreateTestRepository(bare, repo, file, content) + defer func() { + cleanUp() + bare = oldBare + }() + c.Assert(errCreate, gocheck.IsNil) + refs, err := GetForEachRef("gandalf-test-repo", "non_existent_pattern") + c.Assert(err, gocheck.IsNil) + c.Assert(refs, gocheck.HasLen, 1) + c.Assert(refs[0], gocheck.HasLen, 0) +} + +func (s *S) TestGetForEachRefIntegrationWhenInvalidRepo(c *gocheck.C) { + _, err := GetForEachRef("invalid-repo", "refs/") + c.Assert(err.Error(), gocheck.Equals, "Error when trying to obtain the refs of repository invalid-repo (Repository does not exist).") +} + +func (s *S) TestGetForEachRefIntegrationWhenPatternSpaced(c *gocheck.C) { oldBare := bare bare = "/tmp" repo := "gandalf-test-repo" @@ -924,9 +965,10 @@ func (s *S) TestGetForEachRefIntegrationWhenPatternEmpty(c *gocheck.C) { c.Assert(errCreate, gocheck.IsNil) errCreateBranches := CreateBranchesOnTestRepository(bare, repo, "doge_howls") c.Assert(errCreateBranches, gocheck.IsNil) - refs, err := GetForEachRef("gandalf-test-repo", "") + refs, err := GetForEachRef("gandalf-test-repo", "much bark") c.Assert(err, gocheck.IsNil) - c.Assert(len(refs), gocheck.Equals, 2) + c.Assert(refs, gocheck.HasLen, 1) + c.Assert(refs[0], gocheck.HasLen, 0) } func (s *S) TestGetForEachRefIntegrationWhenPatternInvalid(c *gocheck.C) { @@ -941,7 +983,6 @@ func (s *S) TestGetForEachRefIntegrationWhenPatternInvalid(c *gocheck.C) { bare = oldBare }() c.Assert(errCreate, gocheck.IsNil) - refs, err := GetForEachRef("gandalf-test-repo", "invalid_pattern") - c.Assert(err, gocheck.IsNil) - c.Assert(len(refs), gocheck.Equals, 1) + _, err := GetForEachRef("gandalf-test-repo", "--format") + c.Assert(err.Error(), gocheck.Equals, "Error when trying to obtain the refs of repository gandalf-test-repo (exit status 129).") } From 0e85330755492c52ccf56e7fae48bec1810d51f3 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Fri, 25 Jul 2014 13:47:55 -0300 Subject: [PATCH 09/10] Fix minimum number of expected fields --- repository/repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository/repository.go b/repository/repository.go index 9446996..cd7067f 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -381,7 +381,7 @@ func (*GitContentRetriever) GetForEachRef(repo, pattern string) ([]map[string]st continue } fields := strings.Split(line, "\t") - if len(fields) > 4 { // let there be commits with empty subject + if len(fields) > 7 { // let there be commits with empty subject ref = fields[0] name = fields[1] commiterName = fields[2] From 7688d538b4add67d30c7c9b3e6a534226b715f27 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Wed, 23 Jul 2014 17:41:18 -0300 Subject: [PATCH 10/10] Add handler for `GET /repository/repo/branch` (fix #123) --- api/handler.go | 22 +++++++++++++ api/handler_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++ docs/source/api.rst | 17 ++++++++++ webserver/main.go | 1 + 4 files changed, 115 insertions(+) diff --git a/api/handler.go b/api/handler.go index 910ddb3..5f533b8 100644 --- a/api/handler.go +++ b/api/handler.go @@ -333,3 +333,25 @@ func GetTree(w http.ResponseWriter, r *http.Request) { } w.Write(b) } + +func GetBranch(w http.ResponseWriter, r *http.Request) { + repo := r.URL.Query().Get(":name") + if repo == "" { + err := fmt.Errorf("Error when trying to obtain the branches of repository %s (repository is required).", repo) + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + branches, err := repository.GetBranch(repo) + if err != nil { + err := fmt.Errorf("Error when trying to obtain the branches of repository %s (%s).", repo, err) + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + b, err := json.Marshal(branches) + if err != nil { + err := fmt.Errorf("Error when trying to obtain the branches of repository %s (%s).", repo, err) + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + w.Write(b) +} diff --git a/api/handler_test.go b/api/handler_test.go index 2c89d08..b0a8e85 100644 --- a/api/handler_test.go +++ b/api/handler_test.go @@ -971,3 +971,78 @@ func (s *S) TestGetTreeWhenCommandFails(c *gocheck.C) { c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest) c.Assert(recorder.Body.String(), gocheck.Equals, "Error when trying to obtain tree for path /test on ref master of repository repo (output error).\n") } + +func (s *S) TestGetBranch(c *gocheck.C) { + url := "/repository/repo/branch?:name=repo" + refs := make([]map[string]string, 1) + refs[0] = make(map[string]string) + refs[0]["ref"] = "a0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9" + refs[0]["name"] = "doge_barks" + refs[0]["commiterName"] = "doge" + refs[0]["commiterEmail"] = "" + refs[0]["authorName"] = "doge" + refs[0]["authorEmail"] = "" + refs[0]["subject"] = "will bark" + mockRetriever := repository.MockContentRetriever{ + Refs: refs, + } + repository.Retriever = &mockRetriever + defer func() { + repository.Retriever = nil + }() + request, err := http.NewRequest("GET", url, nil) + c.Assert(err, gocheck.IsNil) + recorder := httptest.NewRecorder() + GetBranch(recorder, request) + c.Assert(recorder.Code, gocheck.Equals, http.StatusOK) + var obj []map[string]string + json.Unmarshal(recorder.Body.Bytes(), &obj) + c.Assert(len(obj), gocheck.Equals, 1) + c.Assert(obj[0]["ref"], gocheck.Equals, refs[0]["ref"]) + c.Assert(obj[0]["name"], gocheck.Equals, refs[0]["name"]) + c.Assert(obj[0]["commiterName"], gocheck.Equals, refs[0]["commiterName"]) + c.Assert(obj[0]["commiterEmail"], gocheck.Equals, refs[0]["commiterEmail"]) + c.Assert(obj[0]["authorName"], gocheck.Equals, refs[0]["authorName"]) + c.Assert(obj[0]["authorEmail"], gocheck.Equals, refs[0]["authorEmail"]) + c.Assert(obj[0]["subject"], gocheck.Equals, refs[0]["subject"]) +} + +func (s *S) TestGetBranchWhenRepoNotSupplied(c *gocheck.C) { + url := "/repository//branch?:name=" + request, err := http.NewRequest("GET", url, nil) + c.Assert(err, gocheck.IsNil) + recorder := httptest.NewRecorder() + GetBranch(recorder, request) + c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest) + expected := "Error when trying to obtain the branches of repository (repository is required).\n" + c.Assert(recorder.Body.String(), gocheck.Equals, expected) +} + +func (s *S) TestGetBranchWhenRepoNonExistent(c *gocheck.C) { + url := "/repository/repo/branch?:name=repo" + request, err := http.NewRequest("GET", url, nil) + c.Assert(err, gocheck.IsNil) + recorder := httptest.NewRecorder() + GetBranch(recorder, request) + c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest) + expected := "Error when trying to obtain the branches of repository repo (Error when trying to obtain the refs of repository repo (Repository does not exist).).\n" + c.Assert(recorder.Body.String(), gocheck.Equals, expected) +} + +func (s *S) TestGetBranchWhenCommandFails(c *gocheck.C) { + url := "/repository/repo/branch/?:name=repo" + expected := fmt.Errorf("output error") + mockRetriever := repository.MockContentRetriever{ + OutputError: expected, + } + repository.Retriever = &mockRetriever + defer func() { + repository.Retriever = nil + }() + request, err := http.NewRequest("GET", url, nil) + c.Assert(err, gocheck.IsNil) + recorder := httptest.NewRecorder() + GetBranch(recorder, request) + c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest) + c.Assert(recorder.Body.String(), gocheck.Equals, "Error when trying to obtain the branches of repository repo (output error).\n") +} diff --git a/docs/source/api.rst b/docs/source/api.rst index 7914c30..4243ecd 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -129,3 +129,20 @@ Example URLs (http://gandalf-server omitted for clarity):: $ curl /repository/myrepository/archive/master.zip # gets master and zip format $ curl /repository/myrepository/archive/master.tar.gz # gets master and tar.gz format $ curl /repository/myrepository/archive/0.1.0.zip # gets 0.1.0 tag and zip format + +Get branch +----------- + +Returns a list of all the branches of the specified `repository`. + +* Method: GET +* URI: /repository/`:name`/branch +* Format: JSON + +Where: + +* `:name` is the name of the repository. + +Example URL (http://gandalf-server omitted for clarity):: + + $ curl /repository/myrepository/branch # gets list of branches diff --git a/webserver/main.go b/webserver/main.go index d280cc5..5b82bca 100644 --- a/webserver/main.go +++ b/webserver/main.go @@ -49,6 +49,7 @@ For an example conf check gandalf/etc/gandalf.conf file.\n %s` router.Get("/repository/:name/contents", http.HandlerFunc(api.GetFileContents)) router.Get("/repository/:name/tree/:path", http.HandlerFunc(api.GetTree)) router.Get("/repository/:name/tree", http.HandlerFunc(api.GetTree)) + router.Get("/repository/:name/branch", http.HandlerFunc(api.GetTree)) router.Get("/healthcheck/", http.HandlerFunc(api.HealthCheck)) router.Post("/hook/:name", http.HandlerFunc(api.AddHook))