Added repository update support

Note: the Update method, called via API as
`PUT /repository/<name>/update` doesn`t
update the name cause it`s Rename`s job.

api/handler.go

@@ -64,7 +64,6 @@ func SetupRouter() *pat.Router {
 	router.Post("/user", http.HandlerFunc(newUser))
 	router.Delete("/user/{name}", http.HandlerFunc(removeUser))
 	router.Delete("/repository/revoke", http.HandlerFunc(revokeAccess))
-	router.Put("/repository/set", http.HandlerFunc(setAccess))
 	router.Get("/repository/{name:[^/]*/?[^/]+}/archive", http.HandlerFunc(getArchive))
 	router.Get("/repository/{name:[^/]*/?[^/]+}/contents", http.HandlerFunc(getFileContents))
 	router.Get("/repository/{name:[^/]*/?[^/]+}/tree", http.HandlerFunc(getTree))
@@ -77,30 +76,13 @@ func SetupRouter() *pat.Router {
 	router.Post("/repository", http.HandlerFunc(newRepository))
 	router.Get("/repository/{name:[^/]*/?[^/]+}", http.HandlerFunc(getRepository))
 	router.Delete("/repository/{name:[^/]*/?[^/]+}", http.HandlerFunc(removeRepository))
+	router.Put("/repository/{name:[^/]*/?[^/]+}/update", http.HandlerFunc(updateRepository))
 	router.Put("/repository/{name:[^/]*/?[^/]+}", http.HandlerFunc(renameRepository))
 	router.Get("/healthcheck", http.HandlerFunc(healthCheck))
 	router.Post("/hook/{name}", http.HandlerFunc(addHook))
 	return router
 }
 
-func setAccess(w http.ResponseWriter, r *http.Request) {
-	repositories, users, err := accessParameters(r.Body)
-	readOnly := r.URL.Query().Get("readonly") == "yes"
-	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
-	}
-	if err := repository.SetAccess(repositories, users, readOnly); err != nil {
-		http.Error(w, err.Error(), http.StatusNotFound)
-		return
-	}
-	if readOnly {
-		fmt.Fprintf(w, "Successfully set read-only access to users \"%s\" into repository \"%s\"", users, repositories)
-	} else {
-		fmt.Fprintf(w, "Successfully set full access to users \"%s\" into repository \"%s\"", users, repositories)
-	}
-}
-
 func grantAccess(w http.ResponseWriter, r *http.Request) {
 	repositories, users, err := accessParameters(r.Body)
 	readOnly := r.URL.Query().Get("readonly") == "yes"
@@ -253,6 +235,26 @@ func removeRepository(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprintf(w, "Repository \"%s\" successfully removed\n", name)
 }
 
+func updateRepository(w http.ResponseWriter, r *http.Request) {
+	name := r.URL.Query().Get(":name")
+	repo, err := repository.Get(name)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusNotFound)
+		return
+	}
+	defer r.Body.Close()
+	err = parseBody(r.Body, &repo)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+	}
+	err = repository.Update(name, repo)
+	if err != nil && err.Error() == "not found" {
+		http.Error(w, err.Error(), http.StatusNotFound)
+	} else if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+	}
+}
+
 func renameRepository(w http.ResponseWriter, r *http.Request) {
 	var p struct{ Name string }
 	defer r.Body.Close()

api/handler_test.go

@@ -309,6 +309,19 @@ func (s *S) TestParseBodyShouldMapBodyJsonToGivenStruct(c *gocheck.C) {
 	c.Assert(p.Name, gocheck.Equals, expected)
 }
 
+func (s *S) TestParseBodyShouldMapBodyEmptyJsonToADict(c *gocheck.C) {
+	dict := make(map[string]interface{})
+	b := bufferCloser{bytes.NewBufferString(`{"name": "Test", "isPublic": false, "users": []}`)}
+	err := parseBody(b, &dict)
+	c.Assert(err, gocheck.IsNil)
+	expected := map[string]interface{}{
+		"name":     "Test",
+		"isPublic": false,
+		"users":    []interface{}{},
+	}
+	c.Assert(dict, gocheck.DeepEquals, expected)
+}
+
 func (s *S) TestParseBodyShouldReturnErrorWhenJsonIsInvalid(c *gocheck.C) {
 	var p repository.Repository
 	b := bufferCloser{bytes.NewBufferString("{]ja9aW}")}
@@ -342,67 +355,6 @@ func (s *S) TestNewRepositoryShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
 	c.Assert(recorder.Code, gocheck.Equals, 400)
 }
 
-func (s *S) TestSetAccessShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
-	b := strings.NewReader("")
-	recorder, request := put("/repository/set", b, c)
-	s.router.ServeHTTP(recorder, request)
-	c.Assert(recorder.Code, gocheck.Equals, 400)
-}
-
-func (s *S) TestSetAccessUpdatesReposDocument(c *gocheck.C) {
-	u, err := user.New("pippin", map[string]string{})
-	conn, err := db.Conn()
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Close()
-	defer conn.User().Remove(bson.M{"_id": "pippin"})
-	c.Assert(err, gocheck.IsNil)
-	r := repository.Repository{Name: "onerepo", Users: []string{"oneuser"}}
-	err = conn.Repository().Insert(&r)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().Remove(bson.M{"_id": r.Name})
-	r2 := repository.Repository{Name: "otherepo", Users: []string{"otheruser"}}
-	err = conn.Repository().Insert(&r2)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().Remove(bson.M{"_id": r2.Name})
-	b := bytes.NewBufferString(fmt.Sprintf(`{"repositories": ["%s", "%s"], "users": ["%s"]}`, r.Name, r2.Name, u.Name))
-	rec, req := put("/repository/set", b, c)
-	s.router.ServeHTTP(rec, req)
-	var repos []repository.Repository
-	err = conn.Repository().Find(bson.M{"_id": bson.M{"$in": []string{r.Name, r2.Name}}}).All(&repos)
-	c.Assert(err, gocheck.IsNil)
-	c.Assert(rec.Code, gocheck.Equals, 200)
-	for _, repo := range repos {
-		c.Assert(repo.Users, gocheck.DeepEquals, []string{u.Name})
-	}
-}
-
-func (s *S) TestSetReadonlyAccessUpdatesReposDocument(c *gocheck.C) {
-	u, err := user.New("pippin", map[string]string{})
-	conn, err := db.Conn()
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Close()
-	defer conn.User().Remove(bson.M{"_id": "pippin"})
-	c.Assert(err, gocheck.IsNil)
-	r := repository.Repository{Name: "onerepo", ReadOnlyUsers: []string{"oneuser"}}
-	err = conn.Repository().Insert(&r)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().Remove(bson.M{"_id": r.Name})
-	r2 := repository.Repository{Name: "otherepo", ReadOnlyUsers: []string{"otheruser"}}
-	err = conn.Repository().Insert(&r2)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().Remove(bson.M{"_id": r2.Name})
-	b := bytes.NewBufferString(fmt.Sprintf(`{"repositories": ["%s", "%s"], "users": ["%s"]}`, r.Name, r2.Name, u.Name))
-	rec, req := put("/repository/set?readonly=yes", b, c)
-	s.router.ServeHTTP(rec, req)
-	var repos []repository.Repository
-	err = conn.Repository().Find(bson.M{"_id": bson.M{"$in": []string{r.Name, r2.Name}}}).All(&repos)
-	c.Assert(err, gocheck.IsNil)
-	c.Assert(rec.Code, gocheck.Equals, 200)
-	for _, repo := range repos {
-		c.Assert(repo.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
-	}
-}
-
 func (s *S) TestGrantAccessUpdatesReposDocument(c *gocheck.C) {
 	u, err := user.New("pippin", map[string]string{})
 	conn, err := db.Conn()
@@ -949,24 +901,52 @@ func (s *S) TestRemoveRepositoryShouldReturnErrorMsgWhenRepoDoesNotExist(c *goch
 	c.Assert(string(b), gocheck.Equals, "Could not remove repository: not found\n")
 }
 
-func (s *S) TestRenameRepository(c *gocheck.C) {
-	r, err := repository.New("raising", []string{"guardian@what.com"}, []string{""}, true)
+func (s *S) TestUpdateRespositoryShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
+	b := strings.NewReader("")
+	recorder, request := put("/repository/something/update", b, c)
+	s.router.ServeHTTP(recorder, request)
+	c.Assert(recorder.Code, gocheck.Equals, 400)
+}
+
+func (s *S) TestUpdateRepositoryData(c *gocheck.C) {
+	r, err := repository.New("something", []string{"guardian@what.com"}, []string{""}, true)
 	c.Assert(err, gocheck.IsNil)
-	url := fmt.Sprintf("/repository/%s", r.Name)
-	body := strings.NewReader(`{"name":"freedom"}`)
+	url := fmt.Sprintf("/repository/%s/update", r.Name)
+	body := strings.NewReader(`{"users": ["b"], "readonlyusers": ["a"], "ispublic": false}`)
 	request, err := http.NewRequest("PUT", url, body)
 	c.Assert(err, gocheck.IsNil)
 	recorder := httptest.NewRecorder()
 	s.router.ServeHTTP(recorder, request)
 	c.Assert(recorder.Code, gocheck.Equals, http.StatusOK)
-	_, err = repository.Get("raising")
-	c.Assert(err, gocheck.NotNil)
-	r.Name = "freedom"
-	repo, err := repository.Get("freedom")
+	r.Users = []string{"b"}
+	r.ReadOnlyUsers = []string{"a"}
+	r.IsPublic = false
+	repo, err := repository.Get("something")
 	c.Assert(err, gocheck.IsNil)
 	c.Assert(repo, gocheck.DeepEquals, *r)
 }
 
+func (s *S) TestUpdateRepositoryNotFound(c *gocheck.C) {
+	url := "/repository/foo/update"
+	body := strings.NewReader(`{"ispublic":true}`)
+	request, err := http.NewRequest("PUT", url, body)
+	c.Assert(err, gocheck.IsNil)
+	recorder := httptest.NewRecorder()
+	s.router.ServeHTTP(recorder, request)
+	c.Assert(recorder.Code, gocheck.Equals, http.StatusNotFound)
+}
+
+func (s *S) TestUpdateRepositoryInvalidJSON(c *gocheck.C) {
+	_, err := repository.New("bar", []string{"guardian@what.com"}, []string{""}, true)
+	url := "/repository/bar/update"
+	body := strings.NewReader(`{"name""`)
+	request, err := http.NewRequest("PUT", url, body)
+	c.Assert(err, gocheck.IsNil)
+	recorder := httptest.NewRecorder()
+	s.router.ServeHTTP(recorder, request)
+	c.Assert(recorder.Code, gocheck.Equals, http.StatusBadRequest)
+}
+
 func (s *S) TestRenameRepositoryWithNamespace(c *gocheck.C) {
 	r, err := repository.New("lift/raising", []string{"guardian@what.com"}, []string{}, true)
 	c.Assert(err, gocheck.IsNil)

repository/repository.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"strings"
 
@@ -178,6 +179,40 @@ func Remove(name string) error {
 	return nil
 }
 
+// Update update a repository data.
+func Update(name string, newData Repository) error {
+	log.Debugf("Updating repository %q data", name)
+	repo, err := Get(name)
+	if err != nil {
+		log.Errorf("repository.Update: Repository %q not found: %s", name, err)
+		return err
+	}
+	d := bson.M{}
+	if !reflect.DeepEqual(newData.Users, repo.Users) {
+		d["users"] = newData.Users
+	}
+	if !reflect.DeepEqual(newData.ReadOnlyUsers, repo.ReadOnlyUsers) {
+		d["readonlyusers"] = newData.ReadOnlyUsers
+	}
+	if !reflect.DeepEqual(newData.IsPublic, repo.IsPublic) {
+		d["ispublic"] = newData.IsPublic
+	}
+	if len(d) == 0 {
+		return nil
+	}
+	conn, err := db.Conn()
+	if err != nil {
+		return err
+	}
+	defer conn.Close()
+	err = conn.Repository().UpdateId(repo.Name, bson.M{"$set": d})
+	if err != nil {
+		log.Errorf("repository.Update: Error updating repository data %q: %s", repo.Name, err)
+		return err
+	}
+	return nil
+}
+
 // Rename renames a repository.
 func Rename(oldName, newName string) error {
 	log.Debugf("Renaming repository %q to %q", oldName, newName)
@@ -285,22 +320,6 @@ func (r *Repository) isValid() (bool, error) {
 	return true, nil
 }
 
-// SetAccess gives full or read-only permission for users in all specified repositories.
-// It redefines all users permissions, replacing the respective user collection
-func SetAccess(rNames, uNames []string, readOnly bool) error {
-	conn, err := db.Conn()
-	if err != nil {
-		return err
-	}
-	defer conn.Close()
-	if readOnly {
-		_, err = conn.Repository().UpdateAll(bson.M{"_id": bson.M{"$in": rNames}}, bson.M{"$set": bson.M{"readonlyusers": uNames}})
-	} else {
-		_, err = conn.Repository().UpdateAll(bson.M{"_id": bson.M{"$in": rNames}}, bson.M{"$set": bson.M{"users": uNames}})
-	}
-	return err
-}
-
 // GrantAccess gives full or read-only permission for users in all specified repositories.
 // If any of the repositories/users does not exist, GrantAccess just skips it.
 func GrantAccess(rNames, uNames []string, readOnly bool) error {

repository/repository_test.go

@@ -389,6 +389,28 @@ func (s *S) TestRemoveShouldReturnMeaningfulErrorWhenRepositoryDoesNotExistInDat
 	c.Assert(err, gocheck.ErrorMatches, "^Could not remove repository: not found$")
 }
 
+func (s *S) TestUpdate(c *gocheck.C) {
+	tmpdir, err := commandmocker.Add("git", "$*")
+	c.Assert(err, gocheck.IsNil)
+	r, err := New("freedom", []string{"c"}, []string{"d"}, false)
+	c.Check(err, gocheck.IsNil)
+	commandmocker.Remove(tmpdir)
+	rfs := &fstesting.RecordingFs{}
+	fs.Fsystem = rfs
+	defer func() { fs.Fsystem = nil }()
+	expected := Repository{
+		Name:          "freedom",
+		Users:         []string{"a", "b"},
+		ReadOnlyUsers: []string{"c", "d"},
+		IsPublic:      true,
+	}
+	err = Update(r.Name, expected)
+	c.Assert(err, gocheck.IsNil)
+	repo, err := Get("freedom")
+	c.Assert(err, gocheck.IsNil)
+	c.Assert(repo, gocheck.DeepEquals, expected)
+}
+
 func (s *S) TestRename(c *gocheck.C) {
 	tmpdir, err := commandmocker.Add("git", "$*")
 	c.Assert(err, gocheck.IsNil)
@@ -512,68 +534,6 @@ func (s *S) TestReadWriteURLUseUidFromConfigFile(c *gocheck.C) {
 	c.Assert(remote, gocheck.Equals, fmt.Sprintf("test@%s:f#.git", host))
 }
 
-func (s *S) TestSetAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
-	tmpdir, err := commandmocker.Add("git", "$*")
-	c.Assert(err, gocheck.IsNil)
-	defer commandmocker.Remove(tmpdir)
-	r, err := New("proj1", []string{"someuser"}, []string{"otheruser"}, true)
-	c.Assert(err, gocheck.IsNil)
-	conn, err := db.Conn()
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Close()
-	defer conn.Repository().RemoveId(r.Name)
-	r2, err := New("proj2", []string{"otheruser"}, []string{"someuser"}, true)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().RemoveId(r2.Name)
-	u := struct {
-		Name string `bson:"_id"`
-	}{Name: "lolcat"}
-	err = conn.User().Insert(&u)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.User().RemoveId(u.Name)
-	err = SetAccess([]string{r.Name, r2.Name}, []string{u.Name}, false)
-	c.Assert(err, gocheck.IsNil)
-	err = conn.Repository().FindId(r.Name).One(&r)
-	c.Assert(err, gocheck.IsNil)
-	err = conn.Repository().FindId(r2.Name).One(&r2)
-	c.Assert(err, gocheck.IsNil)
-	c.Assert(r.Users, gocheck.DeepEquals, []string{u.Name})
-	c.Assert(r2.Users, gocheck.DeepEquals, []string{u.Name})
-	c.Assert(r.ReadOnlyUsers, gocheck.DeepEquals, []string{"otheruser"})
-	c.Assert(r2.ReadOnlyUsers, gocheck.DeepEquals, []string{"someuser"})
-}
-
-func (s *S) TestSetReadonlyAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
-	tmpdir, err := commandmocker.Add("git", "$*")
-	c.Assert(err, gocheck.IsNil)
-	defer commandmocker.Remove(tmpdir)
-	r, err := New("proj1", []string{"someuser"}, []string{"otheruser"}, true)
-	c.Assert(err, gocheck.IsNil)
-	conn, err := db.Conn()
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Close()
-	defer conn.Repository().RemoveId(r.Name)
-	r2, err := New("proj2", []string{"otheruser"}, []string{"someuser"}, true)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.Repository().RemoveId(r2.Name)
-	u := struct {
-		Name string `bson:"_id"`
-	}{Name: "lolcat"}
-	err = conn.User().Insert(&u)
-	c.Assert(err, gocheck.IsNil)
-	defer conn.User().RemoveId(u.Name)
-	err = SetAccess([]string{r.Name, r2.Name}, []string{u.Name}, true)
-	c.Assert(err, gocheck.IsNil)
-	err = conn.Repository().FindId(r.Name).One(&r)
-	c.Assert(err, gocheck.IsNil)
-	err = conn.Repository().FindId(r2.Name).One(&r2)
-	c.Assert(err, gocheck.IsNil)
-	c.Assert(r.Users, gocheck.DeepEquals, []string{"someuser"})
-	c.Assert(r2.Users, gocheck.DeepEquals, []string{"otheruser"})
-	c.Assert(r.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
-	c.Assert(r2.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
-}
-
 func (s *S) TestGrantAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
 	tmpdir, err := commandmocker.Add("git", "$*")
 	c.Assert(err, gocheck.IsNil)