Merge pull request #168 from ricardodani/master

Added support to update repository data

Author: andrewsmedina

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,12 @@ 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:[^/]*/?[^/]+}", http.HandlerFunc(renameRepository))
+	router.Put("/repository/{name:[^/]*/?[^/]+}", http.HandlerFunc(updateRepository))
 	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,15 +234,19 @@ func removeRepository(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprintf(w, "Repository \"%s\" successfully removed\n", name)
 }
 
-func renameRepository(w http.ResponseWriter, r *http.Request) {
-	var p struct{ Name string }
+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, &p)
+	err = parseBody(r.Body, &repo)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 	}
-	name := r.URL.Query().Get(":name")
-	err = repository.Rename(name, p.Name)
+	err = repository.Update(name, repo)
 	if err != nil && err.Error() == "not found" {
 		http.Error(w, err.Error(), http.StatusNotFound)
 	} else if err != nil {

api/handler_test.go

@@ -309,6 +309,37 @@ 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) TestParseBodyShouldMapBodyJsonAndUpdateMap(c *gocheck.C) {
+	dict := map[string]interface{}{
+		"isPublic":      false,
+		"users":         []string{"merry"},
+		"readonlyusers": []string{"pippin"},
+	}
+	b := bufferCloser{bytes.NewBufferString(`{"name": "Test", "users": []}`)}
+	err := parseBody(b, &dict)
+	c.Assert(err, gocheck.IsNil)
+	expected := map[string]interface{}{
+		"name":          "Test",
+		"isPublic":      false,
+		"users":         []interface{}{},
+		"readonlyusers": []string{"pippin"},
+	}
+	c.Assert(dict, gocheck.DeepEquals, expected)
+}
+
 func (s *S) TestParseBodyShouldReturnErrorWhenJsonIsInvalid(c *gocheck.C) {
 	var p repository.Repository
 	b := bufferCloser{bytes.NewBufferString("{]ja9aW}")}
@@ -342,67 +373,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,27 +919,96 @@ 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{"[email protected]"}, []string{""}, true)
+func (s *S) TestUpdateRespositoryShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
+	r, err := repository.New("something", []string{"[email protected]"}, []string{""}, true)
+	c.Assert(err, gocheck.IsNil)
+	conn, err := db.Conn()
 	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
+	b := strings.NewReader("")
+	recorder, request := put("/repository/something", 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{"[email protected]"}, []string{""}, true)
+	c.Assert(err, gocheck.IsNil)
+	conn, err := db.Conn()
+	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
 	url := fmt.Sprintf("/repository/%s", r.Name)
-	body := strings.NewReader(`{"name":"freedom"}`)
+	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) TestUpdateRepositoryDataPartial(c *gocheck.C) {
+	r, err := repository.New("something", []string{"pippin"}, []string{"merry"}, true)
+	c.Assert(err, gocheck.IsNil)
+	conn, err := db.Conn()
+	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
+	url := fmt.Sprintf("/repository/%s", r.Name)
+	body := strings.NewReader(`{"readonlyusers": ["a", "b"]}`)
+	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)
+	r.Users = []string{"pippin"}
+	r.ReadOnlyUsers = []string{"a", "b"}
+	r.IsPublic = true
+	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"
+	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) {
+	r, err := repository.New("bar", []string{"[email protected]"}, []string{""}, true)
+	c.Assert(err, gocheck.IsNil)
+	conn, err := db.Conn()
+	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
+	url := "/repository/bar"
+	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{"[email protected]"}, []string{}, true)
 	c.Assert(err, gocheck.IsNil)
+	conn, err := db.Conn()
+	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
 	url := fmt.Sprintf("/repository/%s/", r.Name)
 	body := strings.NewReader(`{"name":"norestraint/freedom"}`)
 	request, err := http.NewRequest("PUT", url, body)
@@ -986,6 +1025,12 @@ func (s *S) TestRenameRepositoryWithNamespace(c *gocheck.C) {
 }
 
 func (s *S) TestRenameRepositoryInvalidJSON(c *gocheck.C) {
+	r, err := repository.New("foo", []string{"[email protected]"}, []string{}, true)
+	conn, err := db.Conn()
+	c.Assert(err, gocheck.IsNil)
+	defer conn.Close()
+	defer conn.Repository().RemoveId(r.Name)
+	c.Assert(err, gocheck.IsNil)
 	url := "/repository/foo"
 	body := strings.NewReader(`{"name""`)
 	request, err := http.NewRequest("PUT", url, body)

repository/repository.go

@@ -178,32 +178,45 @@ func Remove(name string) error {
 	return nil
 }
 
-// Rename renames a repository.
-func Rename(oldName, newName string) error {
-	log.Debugf("Renaming repository %q to %q", oldName, newName)
-	repo, err := Get(oldName)
+// 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.Rename: Repository %q not found: %s", oldName, err)
+		log.Errorf("repository.Update: Repository %q not found: %s", name, err)
 		return err
 	}
-	newRepo := repo
-	newRepo.Name = newName
 	conn, err := db.Conn()
 	if err != nil {
 		return err
 	}
 	defer conn.Close()
-	err = conn.Repository().Insert(newRepo)
-	if err != nil {
-		log.Errorf("repository.Rename: Error adding new repository %q: %s", newName, err)
-		return err
-	}
-	err = conn.Repository().RemoveId(oldName)
-	if err != nil {
-		log.Errorf("repository.Rename: Error removing old repository %q: %s", oldName, err)
-		return err
+	if len(newData.Name) > 0 && newData.Name != repo.Name {
+		oldName := repo.Name
+		log.Debugf("Renaming repository %q to %q", oldName, newData.Name)
+		err = conn.Repository().Insert(newData)
+		if err != nil {
+			log.Errorf("repository.Rename: Error adding new repository %q: %s", newData.Name, err)
+			return err
+		}
+		err = conn.Repository().RemoveId(oldName)
+		if err != nil {
+			log.Errorf("repository.Rename: Error removing old repository %q: %s", oldName, err)
+			return err
+		}
+		err = fs.Filesystem().Rename(barePath(oldName), barePath(newData.Name))
+		if err != nil {
+			log.Errorf("repository.Rename: Error renaming old repository in filesystem %q: %s", oldName, err)
+			return err
+		}
+	} else {
+		err = conn.Repository().UpdateId(repo.Name, newData)
+		if err != nil {
+			log.Errorf("repository.Update: Error updating repository data %q: %s", repo.Name, err)
+			return err
+		}
 	}
-	return fs.Filesystem().Rename(barePath(oldName), barePath(newName))
+	return nil
 }
 
 // ReadWriteURL formats the git ssh url and return it. If no remote is configured in
@@ -285,22 +298,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 {