From 6e3dba2cc559275b5287673957ec855e61b4163a Mon Sep 17 00:00:00 2001 From: Unknown Date: Mon, 5 May 2014 19:58:13 -0400 Subject: [PATCH] Clean repo code --- cmd/web.go | 11 +++-- modules/auth/admin.go | 15 ------- modules/auth/auth.go | 14 ------ modules/auth/authentication.go | 15 ------- modules/auth/repo.go | 57 +++++++++++------------- modules/middleware/binding/binding.go | 5 ++- modules/middleware/repo.go | 15 +++++++ routers/admin/admin.go | 32 +++++++------- routers/admin/user.go | 15 +++---- routers/repo/repo.go | 3 +- routers/repo/setting.go | 64 +++++++-------------------- routers/user/user.go | 2 +- templates/admin/nav.tmpl | 2 +- 13 files changed, 96 insertions(+), 154 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index f81512554a..855ce7721c 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -130,7 +130,7 @@ func runWeb(*cli.Context) { m.Get("/user/:username", ignSignIn, user.Profile) m.Group("/repo", func(r martini.Router) { - r.Get("/create", repo.Create) // TODO + r.Get("/create", repo.Create) r.Post("/create", bindIgnErr(auth.CreateRepoForm{}), repo.CreatePost) r.Get("/migrate", repo.Migrate) r.Post("/migrate", bindIgnErr(auth.MigrateRepoForm{}), repo.MigratePost) @@ -165,14 +165,19 @@ func runWeb(*cli.Context) { m.Get("/template/**", dev.TemplatePreview) } + reqOwner := middleware.RequireOwner + m.Group("/:username/:reponame", func(r martini.Router) { r.Get("/settings", repo.Setting) - r.Post("/settings", repo.SettingPost) + r.Post("/settings", bindIgnErr(auth.RepoSettingForm{}), repo.SettingPost) r.Get("/settings/collaboration", repo.Collaboration) r.Post("/settings/collaboration", repo.CollaborationPost) - r.Get("/settings/hooks", repo.WebHooks) + r.Get("/settings/hooks", repo.WebHooks) // TODO r.Get("/settings/hooks/add", repo.WebHooksAdd) r.Get("/settings/hooks/id", repo.WebHooksEdit) + }, reqSignIn, middleware.RepoAssignment(true), reqOwner()) + + m.Group("/:username/:reponame", func(r martini.Router) { r.Get("/action/:action", repo.Action) r.Get("/issues/new", repo.CreateIssue) r.Post("/issues/new", bindIgnErr(auth.CreateIssueForm{}), repo.CreateIssuePost) diff --git a/modules/auth/admin.go b/modules/auth/admin.go index 39d2d09620..02a4dc489a 100644 --- a/modules/auth/admin.go +++ b/modules/auth/admin.go @@ -11,7 +11,6 @@ import ( "github.com/go-martini/martini" "github.com/gogits/gogs/modules/base" - "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/middleware/binding" ) @@ -36,20 +35,6 @@ func (f *AdminEditUserForm) Name(field string) string { } func (f *AdminEditUserForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { - if req.Method == "GET" || errors.Count() == 0 { - return - } - data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) - data["HasError"] = true - AssignForm(f, data) - - if len(errors.Overall) > 0 { - for _, err := range errors.Overall { - log.Error("AdminEditUserForm.Validate: %v", err) - } - return - } - validate(errors, data, f) } diff --git a/modules/auth/auth.go b/modules/auth/auth.go index a0e00c10c5..a7b281e4c9 100644 --- a/modules/auth/auth.go +++ b/modules/auth/auth.go @@ -183,20 +183,6 @@ func (f *InstallForm) Name(field string) string { } func (f *InstallForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { - if req.Method == "GET" || errors.Count() == 0 { - return - } - data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) - data["HasError"] = true - AssignForm(f, data) - - if len(errors.Overall) > 0 { - for _, err := range errors.Overall { - log.Error("InstallForm.Validate: %v", err) - } - return - } - validate(errors, data, f) } diff --git a/modules/auth/authentication.go b/modules/auth/authentication.go index 376a52e9a7..51a9469cbd 100644 --- a/modules/auth/authentication.go +++ b/modules/auth/authentication.go @@ -11,7 +11,6 @@ import ( "github.com/go-martini/martini" "github.com/gogits/gogs/modules/base" - "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/middleware/binding" ) @@ -44,20 +43,6 @@ func (f *AuthenticationForm) Name(field string) string { } func (f *AuthenticationForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { - if req.Method == "GET" || errors.Count() == 0 { - return - } - data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) - data["HasError"] = true - AssignForm(f, data) - - if len(errors.Overall) > 0 { - for _, err := range errors.Overall { - log.Error("AuthenticationForm.Validate: %v", err) - } - return - } - validate(errors, data, f) } diff --git a/modules/auth/repo.go b/modules/auth/repo.go index e61e8202ff..54fa99c83d 100644 --- a/modules/auth/repo.go +++ b/modules/auth/repo.go @@ -11,12 +11,11 @@ import ( "github.com/go-martini/martini" "github.com/gogits/gogs/modules/base" - "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/middleware/binding" ) type CreateRepoForm struct { - RepoName string `form:"repo" binding:"Required;AlphaDash"` + RepoName string `form:"repo" binding:"Required;AlphaDash;MaxSize(100)"` Private bool `form:"private"` Description string `form:"desc" binding:"MaxSize(100)"` Language string `form:"language"` @@ -33,21 +32,7 @@ func (f *CreateRepoForm) Name(field string) string { } func (f *CreateRepoForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { - if req.Method == "GET" || errors.Count() == 0 { - return - } - data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) - data["HasError"] = true - AssignForm(f, data) - - if len(errors.Overall) > 0 { - for _, err := range errors.Overall { - log.Error("CreateRepoForm.Validate: %v", err) - } - return - } - validate(errors, data, f) } @@ -55,7 +40,7 @@ type MigrateRepoForm struct { Url string `form:"url" binding:"Url"` AuthUserName string `form:"auth_username"` AuthPasswd string `form:"auth_password"` - RepoName string `form:"repo" binding:"Required;AlphaDash"` + RepoName string `form:"repo" binding:"Required;AlphaDash;MaxSize(100)"` Mirror bool `form:"mirror"` Private bool `form:"private"` Description string `form:"desc" binding:"MaxSize(100)"` @@ -71,20 +56,30 @@ func (f *MigrateRepoForm) Name(field string) string { } func (f *MigrateRepoForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { - if req.Method == "GET" || errors.Count() == 0 { - return - } - data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) - data["HasError"] = true - AssignForm(f, data) - - if len(errors.Overall) > 0 { - for _, err := range errors.Overall { - log.Error("MigrateRepoForm.Validate: %v", err) - } - return - } - + validate(errors, data, f) +} + +type RepoSettingForm struct { + RepoName string `form:"name" binding:"Required;AlphaDash;MaxSize(100)"` + Description string `form:"desc" binding:"MaxSize(100)"` + Website string `form:"url" binding:"Url;MaxSize(100)"` + Branch string `form:"branch"` + Interval int `form:"interval"` + Private bool `form:"private"` + GoGet bool `form:"goget"` +} + +func (f *RepoSettingForm) Name(field string) string { + names := map[string]string{ + "RepoName": "Repository name", + "Description": "Description", + "Website": "Website address", + } + return names[field] +} + +func (f *RepoSettingForm) Validate(errors *binding.BindingErrors, req *http.Request, context martini.Context) { + data := context.Get(reflect.TypeOf(base.TmplData{})).Interface().(base.TmplData) validate(errors, data, f) } diff --git a/modules/middleware/binding/binding.go b/modules/middleware/binding/binding.go index 6de5955154..93fb51d994 100644 --- a/modules/middleware/binding/binding.go +++ b/modules/middleware/binding/binding.go @@ -267,7 +267,10 @@ func validateStruct(errors *BindingErrors, obj interface{}) { break } case rule == "Url": - if !urlPattern.MatchString(fmt.Sprintf("%v", fieldValue)) { + str := fmt.Sprintf("%v", fieldValue) + if len(str) == 0 { + continue + } else if !urlPattern.MatchString(str) { errors.Fields[field.Name] = BindingUrlError break } diff --git a/modules/middleware/repo.go b/modules/middleware/repo.go index f681ac139a..7d0cc97970 100644 --- a/modules/middleware/repo.go +++ b/modules/middleware/repo.go @@ -7,6 +7,7 @@ package middleware import ( "errors" "fmt" + "net/url" "strings" "github.com/go-martini/martini" @@ -238,3 +239,17 @@ func RepoAssignment(redirect bool, args ...bool) martini.Handler { ctx.Data["IsRepositoryWatching"] = ctx.Repo.IsWatching } } + +func RequireOwner() martini.Handler { + return func(ctx *Context) { + if !ctx.Repo.IsOwner { + if !ctx.IsSigned { + ctx.SetCookie("redirect_to", "/"+url.QueryEscape(ctx.Req.RequestURI)) + ctx.Redirect("/user/login") + return + } + ctx.Handle(404, ctx.Req.RequestURI, nil) + return + } + } +} diff --git a/routers/admin/admin.go b/routers/admin/admin.go index eafe8cb41b..96721bfdeb 100644 --- a/routers/admin/admin.go +++ b/routers/admin/admin.go @@ -112,14 +112,27 @@ func Users(ctx *middleware.Context) { ctx.Data["PageIsUsers"] = true var err error - ctx.Data["Users"], err = models.GetUsers(100, 0) + ctx.Data["Users"], err = models.GetUsers(200, 0) if err != nil { - ctx.Handle(200, "admin.Users", err) + ctx.Handle(500, "admin.Users", err) return } ctx.HTML(200, "admin/users") } +func Repositories(ctx *middleware.Context) { + ctx.Data["Title"] = "Repository Management" + ctx.Data["PageIsRepos"] = true + + var err error + ctx.Data["Repos"], err = models.GetRepos(200, 0) + if err != nil { + ctx.Handle(500, "admin.Repositories", err) + return + } + ctx.HTML(200, "admin/repos") +} + func Auths(ctx *middleware.Context) { ctx.Data["Title"] = "Auth Sources" ctx.Data["PageIsAuths"] = true @@ -127,25 +140,12 @@ func Auths(ctx *middleware.Context) { var err error ctx.Data["Sources"], err = models.GetAuths() if err != nil { - ctx.Handle(200, "admin.Auths", err) + ctx.Handle(500, "admin.Auths", err) return } ctx.HTML(200, "admin/auths") } -func Repositories(ctx *middleware.Context) { - ctx.Data["Title"] = "Repository Management" - ctx.Data["PageIsRepos"] = true - - var err error - ctx.Data["Repos"], err = models.GetRepos(100, 0) - if err != nil { - ctx.Handle(200, "admin.Repositories", err) - return - } - ctx.HTML(200, "admin/repos") -} - func Config(ctx *middleware.Context) { ctx.Data["Title"] = "Server Configuration" ctx.Data["PageIsConfig"] = true diff --git a/routers/admin/user.go b/routers/admin/user.go index f2e1b04730..0d60ee9a76 100644 --- a/routers/admin/user.go +++ b/routers/admin/user.go @@ -34,19 +34,18 @@ func NewUserPost(ctx *middleware.Context, form auth.RegisterForm) { ctx.Data["Title"] = "New Account" ctx.Data["PageIsUsers"] = true - if form.Password != form.RetypePasswd { - ctx.Data["HasError"] = true - ctx.Data["Err_Password"] = true - ctx.Data["Err_RetypePasswd"] = true - ctx.Data["ErrorMsg"] = "Password and re-type password are not same" - auth.AssignForm(form, ctx.Data) - } - if ctx.HasError() { ctx.HTML(200, "admin/users/new") return } + if form.Password != form.RetypePasswd { + ctx.Data["Err_Password"] = true + ctx.Data["Err_RetypePasswd"] = true + ctx.RenderWithErr("Password and re-type password are not same.", "admin/users/new", &form) + return + } + u := &models.User{ Name: form.UserName, Email: form.Email, diff --git a/routers/repo/repo.go b/routers/repo/repo.go index e82c6ae988..19c9dddc6e 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -8,13 +8,14 @@ import ( "encoding/base64" "errors" "fmt" - "github.com/gogits/git" "path" "path/filepath" "strings" "github.com/go-martini/martini" + "github.com/gogits/git" + "github.com/gogits/gogs/models" "github.com/gogits/gogs/modules/auth" "github.com/gogits/gogs/modules/base" diff --git a/routers/repo/setting.go b/routers/repo/setting.go index b0ec761830..c308739207 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -11,6 +11,7 @@ import ( "github.com/gogits/git" "github.com/gogits/gogs/models" + "github.com/gogits/gogs/modules/auth" "github.com/gogits/gogs/modules/base" "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/mailer" @@ -18,27 +19,22 @@ import ( ) func Setting(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Handle(404, "repo.Setting", nil) - return - } - ctx.Data["IsRepoToolbarSetting"] = true ctx.Data["Title"] = strings.TrimPrefix(ctx.Repo.RepoLink, "/") + " - settings" ctx.HTML(200, "repo/setting") } -func SettingPost(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Error(404) - return - } - +func SettingPost(ctx *middleware.Context, form auth.RepoSettingForm) { ctx.Data["IsRepoToolbarSetting"] = true switch ctx.Query("action") { case "update": - newRepoName := ctx.Query("name") + if ctx.HasError() { + ctx.HTML(200, "repo/setting") + return + } + + newRepoName := form.RepoName // Check if repository name has been changed. if ctx.Repo.Repository.Name != newRepoName { isExist, err := models.IsRepositoryExist(ctx.Repo.Owner, newRepoName) @@ -57,15 +53,15 @@ func SettingPost(ctx *middleware.Context) { ctx.Repo.Repository.Name = newRepoName } - br := ctx.Query("branch") + br := form.Branch if git.IsBranchExist(models.RepoPath(ctx.User.Name, ctx.Repo.Repository.Name), br) { ctx.Repo.Repository.DefaultBranch = br } - ctx.Repo.Repository.Description = ctx.Query("desc") - ctx.Repo.Repository.Website = ctx.Query("site") - ctx.Repo.Repository.IsPrivate = ctx.Query("private") == "on" - ctx.Repo.Repository.IsGoget = ctx.Query("goget") == "on" + ctx.Repo.Repository.Description = form.Description + ctx.Repo.Repository.Website = form.Website + ctx.Repo.Repository.IsPrivate = form.Private + ctx.Repo.Repository.IsGoget = form.GoGet if err := models.UpdateRepository(ctx.Repo.Repository); err != nil { ctx.Handle(404, "repo.SettingPost(update)", err) return @@ -73,12 +69,9 @@ func SettingPost(ctx *middleware.Context) { log.Trace("%s Repository updated: %s/%s", ctx.Req.RequestURI, ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) if ctx.Repo.Repository.IsMirror { - if len(ctx.Query("interval")) > 0 { - var err error - ctx.Repo.Mirror.Interval, err = base.StrTo(ctx.Query("interval")).Int() - if err != nil { - log.Error("repo.SettingPost(get mirror interval): %v", err) - } else if err = models.UpdateMirror(ctx.Repo.Mirror); err != nil { + if form.Interval > 0 { + ctx.Repo.Mirror.Interval = form.Interval + if err := models.UpdateMirror(ctx.Repo.Mirror); err != nil { log.Error("repo.SettingPost(UpdateMirror): %v", err) } } @@ -125,11 +118,6 @@ func SettingPost(ctx *middleware.Context) { } func Collaboration(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Error(404) - return - } - repoLink := strings.TrimPrefix(ctx.Repo.RepoLink, "/") ctx.Data["IsRepoToolbarCollaboration"] = true ctx.Data["Title"] = repoLink + " - collaboration" @@ -166,11 +154,6 @@ func Collaboration(ctx *middleware.Context) { } func CollaborationPost(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Error(404) - return - } - repoLink := strings.TrimPrefix(ctx.Repo.RepoLink, "/") name := strings.ToLower(ctx.Query("collaborator")) if len(name) == 0 || ctx.Repo.Owner.LowerName == name { @@ -215,33 +198,18 @@ func CollaborationPost(ctx *middleware.Context) { } func WebHooks(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Handle(404, "repo.WebHooks", nil) - return - } - ctx.Data["IsRepoToolbarWebHooks"] = true ctx.Data["Title"] = strings.TrimPrefix(ctx.Repo.RepoLink, "/") + " - Web Hooks" ctx.HTML(200, "repo/hooks") } func WebHooksAdd(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Handle(404, "repo.WebHooksAdd", nil) - return - } - ctx.Data["IsRepoToolbarWebHooks"] = true ctx.Data["Title"] = strings.TrimPrefix(ctx.Repo.RepoLink, "/") + " - Add Web Hook" ctx.HTML(200, "repo/hooks_add") } func WebHooksEdit(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { - ctx.Handle(404, "repo.WebHooksEdit", nil) - return - } - ctx.Data["IsRepoToolbarWebHooks"] = true ctx.Data["Title"] = strings.TrimPrefix(ctx.Repo.RepoLink, "/") + " - Web Hook Name" ctx.HTML(200, "repo/hooks_edit") diff --git a/routers/user/user.go b/routers/user/user.go index 7af65fc585..e33353abdb 100644 --- a/routers/user/user.go +++ b/routers/user/user.go @@ -217,7 +217,7 @@ func SignUpPost(ctx *middleware.Context, form auth.RegisterForm) { if form.Password != form.RetypePasswd { ctx.Data["Err_Password"] = true ctx.Data["Err_RetypePasswd"] = true - ctx.RenderWithErr("Password and re-type password are not same", "user/signup", &form) + ctx.RenderWithErr("Password and re-type password are not same.", "user/signup", &form) return } diff --git a/templates/admin/nav.tmpl b/templates/admin/nav.tmpl index f27b8bb24b..5ba4495796 100644 --- a/templates/admin/nav.tmpl +++ b/templates/admin/nav.tmpl @@ -3,7 +3,7 @@
  • Dashboard
  • Users
  • Repositories
    • -
  • Configuration
  • Authentication
    • +
  • Configuration
    • \ No newline at end of file