Merge pull request #162 from scorphus/improvements

Improvements

api/handler.go

@@ -71,7 +71,7 @@ func SetupRouter() *pat.Router {
 	router.Get("/repository/{name:[^/]*/?[^/]+}/tags", http.HandlerFunc(getTags))
 	router.Get("/repository/{name:[^/]*/?[^/]+}/diff/commits", http.HandlerFunc(getDiff))
 	router.Post("/repository/{name:[^/]*/?[^/]+}/commit", http.HandlerFunc(commit))
-	router.Get("/repository/{name:[^/]*/?[^/]+}/logs", http.HandlerFunc(getLog))
+	router.Get("/repository/{name:[^/]*/?[^/]+}/logs", http.HandlerFunc(getLogs))
 	router.Post("/repository/grant", http.HandlerFunc(grantAccess))
 	router.Post("/repository", http.HandlerFunc(newRepository))
 	router.Get("/repository/{name:[^/]*/?[^/]+}", http.HandlerFunc(getRepository))
@@ -508,25 +508,25 @@ func commit(w http.ResponseWriter, r *http.Request) {
 	w.Write(b)
 }
 
-func getLog(w http.ResponseWriter, r *http.Request) {
+func getLogs(w http.ResponseWriter, r *http.Request) {
 	repo := r.URL.Query().Get(":name")
 	ref := r.URL.Query().Get("ref")
 	path := r.URL.Query().Get("path")
 	total, err := strconv.Atoi(r.URL.Query().Get("total"))
 	if err != nil {
-		err := fmt.Errorf("Error when trying to obtain log for ref %s of repository %s (%s).", ref, repo, err)
+		err := fmt.Errorf("Error when trying to obtain logs for ref %s of repository %s (%s).", ref, repo, err)
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
-	logs, err := repository.GetLog(repo, ref, total, path)
+	logs, err := repository.GetLogs(repo, ref, total, path)
 	if err != nil {
-		err := fmt.Errorf("Error when trying to obtain log for ref %s of repository %s (%s).", ref, repo, err)
+		err := fmt.Errorf("Error when trying to obtain logs for ref %s of repository %s (%s).", ref, repo, err)
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
 	}
 	b, err := json.Marshal(logs)
 	if err != nil {
-		err := fmt.Errorf("Error when trying to obtain log for ref %s of repository %s (%s).", ref, repo, err)
+		err := fmt.Errorf("Error when trying to obtain logs for ref %s of repository %s (%s).", ref, repo, err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}

api/handler_test.go

@@ -226,7 +226,7 @@ func (s *S) TestGetRepositoryWithNamespace(c *gocheck.C) {
 }
 
 func (s *S) TestGetRepositoryDoesNotExist(c *gocheck.C) {
-	recorder, request := get("/repository/doesnotexists", nil, c)
+	recorder, request := get("/repository/doesnotexist", nil, c)
 	s.router.ServeHTTP(recorder, request)
 	c.Assert(recorder.Code, gocheck.Equals, 500)
 }
@@ -623,7 +623,7 @@ func (s *S) TestAddInvalidOldFormatHook(c *gocheck.C) {
 	c.Assert(recorder.Code, gocheck.Equals, 400)
 }
 
-func (s *S) TestAddKeyShouldReturnErrorWhenUserDoesNotExists(c *gocheck.C) {
+func (s *S) TestAddKeyShouldReturnErrorWhenUserDoesNotExist(c *gocheck.C) {
 	b := strings.NewReader(`{"key": "a public key"}`)
 	recorder, request := post("/user/Frodo/key", b, c)
 	s.router.ServeHTTP(recorder, request)
@@ -872,7 +872,7 @@ func (s *S) TestRemoveRepositoryShouldReturn400OnFailure(c *gocheck.C) {
 	c.Assert(recorder.Code, gocheck.Equals, 400)
 }
 
-func (s *S) TestRemoveRepositoryShouldReturnErrorMsgWhenRepoDoesNotExists(c *gocheck.C) {
+func (s *S) TestRemoveRepositoryShouldReturnErrorMsgWhenRepoDoesNotExist(c *gocheck.C) {
 	url := "/repository/foo"
 	request, err := http.NewRequest("DELETE", url, nil)
 	c.Assert(err, gocheck.IsNil)
@@ -1509,7 +1509,7 @@ func (s *S) TestPostNewCommitWithEmptyBranch(c *gocheck.C) {
 	c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest)
 }
 
-func (s *S) TestLog(c *gocheck.C) {
+func (s *S) TestLogs(c *gocheck.C) {
 	url := "/repository/repo/logs?ref=HEAD&total=1"
 	objects := repository.GitHistory{}
 	parent := make([]string, 2)
@@ -1551,7 +1551,7 @@ func (s *S) TestLog(c *gocheck.C) {
 	c.Assert(obj.Commits[0], gocheck.DeepEquals, commits[0])
 }
 
-func (s *S) TestLogWithPath(c *gocheck.C) {
+func (s *S) TestLogsWithPath(c *gocheck.C) {
 	url := "/repository/repo/logs?ref=HEAD&total=1&path=README.txt"
 	objects := repository.GitHistory{}
 	parent := make([]string, 2)

api/utils_test.go

@@ -70,7 +70,7 @@ func (s *S) TestGetUserOr404(c *gocheck.C) {
 	c.Assert(rUser.Name, gocheck.Equals, "umi")
 }
 
-func (s *S) TestGetUserOr404ShouldReturn404WhenUserDoesntExists(c *gocheck.C) {
+func (s *S) TestGetUserOr404ShouldReturn404WhenUserDoesntExist(c *gocheck.C) {
 	_, e := getUserOr404("umi")
 	expected := "User umi not found"
 	got := e.Error()

bin/gandalf_test.go

@@ -163,7 +163,7 @@ func (s *S) TestRequestedRepositoryShouldReturnErrorWhenThereIsNoCommandPassedTo
 	c.Assert(err, gocheck.ErrorMatches, "^You've tried to execute some weird command, I'm deliberately denying you to do that, get over it.$")
 }
 
-func (s *S) TestRequestedRepositoryShouldReturnFormatedErrorWhenRepositoryDoesNotExists(c *gocheck.C) {
+func (s *S) TestRequestedRepositoryShouldReturnFormatedErrorWhenRepositoryDoesNotExist(c *gocheck.C) {
 	os.Setenv("SSH_ORIGINAL_COMMAND", "git-receive-pack 'inexistent-repo.git'")
 	defer os.Setenv("SSH_ORIGINAL_COMMAND", "")
 	_, err := requestedRepository()
@@ -248,7 +248,7 @@ func (s *S) TestExecuteActionShouldExecuteGitReceivePackWhenUserHasWritePermissi
 	c.Assert(stdout.String(), gocheck.Equals, expected)
 }
 
-func (s *S) TestExecuteActionShouldNotCallSSH_ORIGINAL_COMMANDWhenUserDoesNotExists(c *gocheck.C) {
+func (s *S) TestExecuteActionShouldNotCallSSH_ORIGINAL_COMMANDWhenUserDoesNotExist(c *gocheck.C) {
 	dir, err := commandmocker.Add("git-receive-pack", "$*")
 	c.Check(err, gocheck.IsNil)
 	defer commandmocker.Remove(dir)
@@ -264,7 +264,7 @@ func (s *S) TestExecuteActionShouldNotCallSSH_ORIGINAL_COMMANDWhenUserDoesNotExi
 	c.Assert(commandmocker.Ran(dir), gocheck.Equals, false)
 }
 
-func (s *S) TestExecuteActionShouldNotCallSSH_ORIGINAL_COMMANDWhenRepositoryDoesNotExists(c *gocheck.C) {
+func (s *S) TestExecuteActionShouldNotCallSSH_ORIGINAL_COMMANDWhenRepositoryDoesNotExist(c *gocheck.C) {
 	dir, err := commandmocker.Add("git-receive-pack", "$*")
 	c.Check(err, gocheck.IsNil)
 	defer commandmocker.Remove(dir)

docs/source/api.rst

@@ -167,8 +167,8 @@ Example URLs (http://gandalf-server omitted for clarity)::
     $ curl /repository/myrepository/archive?ref=master&format=tar.gz     # gets master and tar.gz format
     $ curl /repository/myrepository/archive?ref=0.1.0&format=zip         # gets 0.1.0 tag and zip format
 
-Get branch
------------
+Get branches
+------------
 
 Returns a list of all the branches of the specified `repository`.
 
@@ -207,8 +207,8 @@ Example URL (http://gandalf-server omitted for clarity)::
 
     $ curl /repository/myrepository/branches                  # gets list of branches
 
-Get tag
--------
+Get tags
+--------
 
 Returns a list of all the tags of the specified `repository`.
 
@@ -341,7 +341,7 @@ Logs
 Returns a list of all commits into `repository`.
 
 * Method: GET
-* URI: /repository/`:name`/log?ref=:ref&total=:total
+* URI: /repository/`:name`/logs?ref=:ref&total=:total
 * Format: JSON
 
 Where:

repository/mocks.go

@@ -397,7 +397,7 @@ func (r *MockContentRetriever) CommitZip(repo string, z *multipart.FileHeader, c
 	return &r.Ref, nil
 }
 
-func (r *MockContentRetriever) GetLog(repo, hash string, total int, path string) (*GitHistory, error) {
+func (r *MockContentRetriever) GetLogs(repo, hash string, total int, path string) (*GitHistory, error) {
 	if r.LookPathError != nil {
 		return nil, r.LookPathError
 	}

repository/repository.go

@@ -286,7 +286,7 @@ func (r *Repository) isValid() (bool, error) {
 }
 
 // GrantAccess gives full or read-only permission for users in all specified repositories.
-// If any of the repositories/users do not exists, GrantAccess just skips it.
+// If any of the repositories/users does not exist, GrantAccess just skips it.
 func GrantAccess(rNames, uNames []string, readOnly bool) error {
 	conn, err := db.Conn()
 	if err != nil {
@@ -345,7 +345,7 @@ type ContentRetriever interface {
 	Commit(cloneDir, message string, author GitUser) error
 	Push(cloneDir, branch string) error
 	CommitZip(repo string, z *multipart.FileHeader, c GitCommit) (*Ref, error)
-	GetLog(repo, hash string, total int, path string) (*GitHistory, error)
+	GetLogs(repo, hash string, total int, path string) (*GitHistory, error)
 }
 
 var Retriever ContentRetriever
@@ -718,12 +718,15 @@ func (*GitContentRetriever) CommitZip(repo string, z *multipart.FileHeader, c Gi
 	return nil, fmt.Errorf("Error when trying to commit zip to repository %s, could not check branch: %s", repo, err)
 }
 
-func (*GitContentRetriever) GetLog(repo, hash string, total int, path string) (*GitHistory, error) {
+func (*GitContentRetriever) GetLogs(repo, hash string, total int, path string) (*GitHistory, error) {
+	if hash == "" {
+		hash = "master"
+	}
 	if total < 1 {
 		total = 1
 	}
 	totalPagination := total + 1
-	var last, ref, committerName, committerEmail, committerDate, authorName, authorEmail, authorDate, subject, parent string
+	var last string
 	gitPath, err := exec.LookPath("git")
 	if err != nil {
 		return nil, fmt.Errorf("Error when trying to obtain the log of repository %s (%s).", repo, err)
@@ -754,46 +757,43 @@ func (*GitContentRetriever) GetLog(repo, hash string, total int, path string) (*
 	commits := make([]GitLog, objectCount)
 	objectCount = 0
 	for _, line := range lines {
+		var parent, subject string
 		if strings.TrimSpace(line) == "" {
 			continue
 		}
 		fields := strings.Split(line, "\t")
-		if len(fields) > 8 { // let there be commits with empty subject
-			ref = fields[0]
-			authorName = fields[1]
-			authorEmail = fields[2]
-			authorDate = fields[3]
-			committerName = fields[4]
-			committerEmail = fields[5]
-			committerDate = fields[6]
+		if len(fields) < 7 { // let there be commits with empty subject and no parents
+			return nil, fmt.Errorf("Error when trying to obtain the log of repository %s (Invalid git log output [%s]).", repo, out)
+		}
+		if len(fields) > 8 {
 			parent = 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 log of repository %s (Invalid git log output [%s]).", repo, out)
 		}
 		commit := GitLog{}
-		commit.Ref = ref
+		commit.Ref = fields[0]
 		commit.Subject = subject
-		commit.CreatedAt = authorDate
+		commit.CreatedAt = fields[3]
 		commit.Committer = &GitUser{
-			Name:  committerName,
-			Email: committerEmail,
-			Date:  committerDate,
+			Name:  fields[4],
+			Email: fields[5],
+			Date:  fields[6],
 		}
 		commit.Author = &GitUser{
-			Name:  authorName,
-			Email: authorEmail,
-			Date:  authorDate,
+			Name:  fields[1],
+			Email: fields[2],
+			Date:  fields[3],
 		}
-		parents := strings.Split(parent, " ")
-		parentCount := len(parents)
-		aux := make([]string, parentCount)
-		parentCount = 0
-		for _, item := range parents {
-			aux[parentCount] = item
-			parentCount++
+		if len(parent) > 0 {
+			parents := strings.Split(parent, " ")
+			parentCount := len(parents)
+			aux := make([]string, parentCount)
+			parentCount = 0
+			for _, item := range parents {
+				aux[parentCount] = item
+				parentCount++
+			}
+			commit.Parent = aux
 		}
-		commit.Parent = aux
 		commits[objectCount] = commit
 		objectCount++
 	}
@@ -874,6 +874,6 @@ func CommitZip(repo string, z *multipart.FileHeader, c GitCommit) (*Ref, error)
 	return retriever().CommitZip(repo, z, c)
 }
 
-func GetLog(repo, hash string, total int, path string) (*GitHistory, error) {
-	return retriever().GetLog(repo, hash, total, path)
+func GetLogs(repo, hash string, total int, path string) (*GitHistory, error) {
+	return retriever().GetLogs(repo, hash, total, path)
 }