From b8bcdaa95339911ba3c19b540577d478567ae1e6 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Mon, 11 Aug 2014 18:17:50 -0300 Subject: [PATCH 1/4] Declare maxMemory as uint --- api/handler.go | 6 +++--- api/handler_test.go | 12 ++++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/api/handler.go b/api/handler.go index 48f9ba7..b7c4ab9 100644 --- a/api/handler.go +++ b/api/handler.go @@ -24,14 +24,14 @@ import ( "strings" ) -var maxMemory int +var maxMemory uint -func maxMemoryValue() int { +func maxMemoryValue() uint { if maxMemory > 0 { return maxMemory } var err error - maxMemory, err = config.GetInt("api:request:maxMemory") + maxMemory, err = config.GetUint("api:request:maxMemory") if err != nil { panic("You should configure a api:request:maxMemory for gandalf.") } diff --git a/api/handler_test.go b/api/handler_test.go index b33c27c..e2de3e4 100644 --- a/api/handler_test.go +++ b/api/handler_test.go @@ -67,14 +67,22 @@ func (s *S) authKeysContent(c *gocheck.C) string { func (s *S) TestMaxMemoryValueShouldComeFromGandalfConf(c *gocheck.C) { config.Set("api:request:maxMemory", 1024) + oldMaxMemory := maxMemory maxMemory = 0 - c.Assert(maxMemoryValue(), gocheck.Equals, 1024) + defer func() { + maxMemory = oldMaxMemory + }() + c.Assert(maxMemoryValue(), gocheck.Equals, uint(1024)) } func (s *S) TestMaxMemoryValueDontResetMaxMemory(c *gocheck.C) { config.Set("api:request:maxMemory", 1024) + oldMaxMemory := maxMemory maxMemory = 359 - c.Assert(maxMemoryValue(), gocheck.Equals, 359) + defer func() { + maxMemory = oldMaxMemory + }() + c.Assert(maxMemoryValue(), gocheck.Equals, uint(359)) } func (s *S) TestNewUser(c *gocheck.C) { From ba839f55c78664aed74d4e910c197c2cf43dd04e Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Mon, 11 Aug 2014 18:20:54 -0300 Subject: [PATCH 2/4] Use named return values to ease function usage --- repository/repository.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/repository/repository.go b/repository/repository.go index ece64f7..a31e0ab 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -499,7 +499,7 @@ func (*GitContentRetriever) GetTags(repo string) ([]Ref, error) { return tags, err } -func (*GitContentRetriever) TempClone(repo string) (string, func(), error) { +func (*GitContentRetriever) TempClone(repo string) (cloneDir string, cleanUp func(), err error) { gitPath, err := exec.LookPath("git") if err != nil { return "", nil, fmt.Errorf("Error when trying to clone repository %s (%s).", repo, err) @@ -509,19 +509,19 @@ func (*GitContentRetriever) TempClone(repo string) (string, func(), error) { if err != nil || !repoExists { return "", nil, fmt.Errorf("Error when trying to clone repository %s (Repository does not exist).", repo) } - cloneDir, err := ioutil.TempDir("", "gandalf_clone") + cloneDir, err = ioutil.TempDir("", "gandalf_clone") if err != nil { return "", nil, fmt.Errorf("Error when trying to clone repository %s (Could not create temporary directory).", repo) } - cleanup := func() { + cleanUp = func() { os.RemoveAll(cloneDir) } cmd := exec.Command(gitPath, "clone", repoDir, cloneDir) out, err := cmd.CombinedOutput() if err != nil { - return cloneDir, cleanup, fmt.Errorf("Error when trying to clone repository %s into %s (%s [%s]).", repo, cloneDir, err, out) + return cloneDir, cleanUp, fmt.Errorf("Error when trying to clone repository %s into %s (%s [%s]).", repo, cloneDir, err, out) } - return cloneDir, cleanup, nil + return cloneDir, cleanUp, nil } func (*GitContentRetriever) SetCommitter(cloneDir string, committer GitUser) error { From 66ab52d7c7c4393b94bc1a558a385e2e97faf0fc Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Mon, 11 Aug 2014 18:21:22 -0300 Subject: [PATCH 3/4] Define the temporary directory in gandalf conf --- etc/gandalf.conf | 2 ++ repository/repository.go | 16 +++++++++++++++- repository/repository_test.go | 20 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/etc/gandalf.conf b/etc/gandalf.conf index 3a05c65..14ed547 100644 --- a/etc/gandalf.conf +++ b/etc/gandalf.conf @@ -16,3 +16,5 @@ uid: git api: request: maxMemory: 2048 +repository: + tempDir: /tmp diff --git a/repository/repository.go b/repository/repository.go index a31e0ab..5823d39 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -23,6 +23,20 @@ import ( "strings" ) +var tempDir string + +func tempDirLocation() string { + if tempDir != "" { + return tempDir + } + var err error + tempDir, err = config.GetString("repository:tempDir") + if err != nil { + panic("You should configure a repository:tempDir for gandalf.") + } + return tempDir +} + // Repository represents a Git repository. A Git repository is a record in the // database and a directory in the filesystem (the bare repository). type Repository struct { @@ -509,7 +523,7 @@ func (*GitContentRetriever) TempClone(repo string) (cloneDir string, cleanUp fun if err != nil || !repoExists { return "", nil, fmt.Errorf("Error when trying to clone repository %s (Repository does not exist).", repo) } - cloneDir, err = ioutil.TempDir("", "gandalf_clone") + cloneDir, err = ioutil.TempDir(tempDir, "gandalf_clone") if err != nil { return "", nil, fmt.Errorf("Error when trying to clone repository %s (Could not create temporary directory).", repo) } diff --git a/repository/repository_test.go b/repository/repository_test.go index ffbac5d..13cd233 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -50,6 +50,26 @@ func (s *S) TearDownSuite(c *gocheck.C) { conn.User().Database.DropDatabase() } +func (s *S) TestTempDirLocationShouldComeFromGandalfConf(c *gocheck.C) { + config.Set("repository:tempDir", "/home/gandalf/temp") + oldTempDir := tempDir + tempDir = "" + defer func() { + tempDir = oldTempDir + }() + c.Assert(tempDirLocation(), gocheck.Equals, "/home/gandalf/temp") +} + +func (s *S) TestTempDirLocationDontResetTempDir(c *gocheck.C) { + config.Set("repository:tempDir", "/home/gandalf/temp") + oldTempDir := tempDir + tempDir = "/var/folders" + defer func() { + tempDir = oldTempDir + }() + c.Assert(tempDirLocation(), gocheck.Equals, "/var/folders") +} + func (s *S) TestNewShouldCreateANewRepository(c *gocheck.C) { tmpdir, err := commandmocker.Add("git", "$*") c.Assert(err, gocheck.IsNil) From 91e7234410a98dde48c82847ad495a4319725cb5 Mon Sep 17 00:00:00 2001 From: Pablo Santiago Blum de Aguiar Date: Mon, 11 Aug 2014 18:35:38 -0300 Subject: [PATCH 4/4] Create and use a type instead of an anonymous struct --- api/handler_test.go | 12 +++--------- multipartzip/mocks.go | 7 ++++++- multipartzip/multipartzip_test.go | 12 +++--------- repository/repository_test.go | 4 +--- 4 files changed, 13 insertions(+), 22 deletions(-) diff --git a/api/handler_test.go b/api/handler_test.go index e2de3e4..4bf2d57 100644 --- a/api/handler_test.go +++ b/api/handler_test.go @@ -1303,9 +1303,7 @@ func (s *S) TestPostNewCommit(c *gocheck.C) { "committer-email": "doge@much.com", "branch": "master", } - var files = []struct { - Name, Body string - }{ + var files = []multipartzip.File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW", "WOW\nWOW"}, @@ -1380,9 +1378,7 @@ func (s *S) TestPostNewCommitWithoutBranch(c *gocheck.C) { "committer-name": "Doge Dog", "committer-email": "doge@much.com", } - var files = []struct { - Name, Body string - }{ + var files = []multipartzip.File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW", "WOW\nWOW"}, @@ -1413,9 +1409,7 @@ func (s *S) TestPostNewCommitWithEmptyBranch(c *gocheck.C) { "committer-email": "doge@much.com", "branch": "", } - var files = []struct { - Name, Body string - }{ + var files = []multipartzip.File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW", "WOW\nWOW"}, diff --git a/multipartzip/mocks.go b/multipartzip/mocks.go index ee91746..f536c73 100644 --- a/multipartzip/mocks.go +++ b/multipartzip/mocks.go @@ -13,7 +13,12 @@ import ( "path/filepath" ) -func CreateZipBuffer(files []struct{ Name, Body string }) (*bytes.Buffer, error) { +type File struct { + Name string + Body string +} + +func CreateZipBuffer(files []File) (*bytes.Buffer, error) { buf := new(bytes.Buffer) w := zip.NewWriter(buf) for _, file := range files { diff --git a/multipartzip/multipartzip_test.go b/multipartzip/multipartzip_test.go index 6d687fa..3daed1a 100644 --- a/multipartzip/multipartzip_test.go +++ b/multipartzip/multipartzip_test.go @@ -29,9 +29,7 @@ func (s *S) TestCopyZipFile(c *gocheck.C) { os.RemoveAll(tempDir) }() c.Assert(err, gocheck.IsNil) - var files = []struct { - Name, Body string - }{ + var files = []File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW1", "WOW\nWOW"}, @@ -55,9 +53,7 @@ func (s *S) TestCopyZipFile(c *gocheck.C) { func (s *S) TestExtractZip(c *gocheck.C) { boundary := "muchBOUNDARY" params := map[string]string{} - var files = []struct { - Name, Body string - }{ + var files = []File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW1", "WOW\nWOW"}, @@ -137,9 +133,7 @@ func (s *S) TestValueFieldWhenFieldEmpty(c *gocheck.C) { func (s *S) TestFileField(c *gocheck.C) { boundary := "muchBOUNDARY" params := map[string]string{} - var files = []struct { - Name, Body string - }{ + var files = []File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"WOW/WOW.WOW1", "WOW\nWOW"}, diff --git a/repository/repository_test.go b/repository/repository_test.go index 13cd233..9a0faec 100644 --- a/repository/repository_test.go +++ b/repository/repository_test.go @@ -1635,9 +1635,7 @@ func (s *S) TestCommitZipIntegration(c *gocheck.C) { c.Assert(errCreate, gocheck.IsNil) boundary := "muchBOUNDARY" params := map[string]string{} - var files = []struct { - Name, Body string - }{ + var files = []multipartzip.File{ {"doge.txt", "Much doge"}, {"much.txt", "Much mucho"}, {"much/WOW.txt", "Much WOW"},