From 6235bd1fe9ed15c9a889f72e4fe2880189963306 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Tue, 18 Aug 2015 04:03:11 +0800 Subject: [PATCH] work on #986 and fix a LDAP crash --- models/cron/cron.go | 25 +++++++- models/repo.go | 130 ++++++++++++++++++++++++-------------- modules/auth/ldap/ldap.go | 8 +-- modules/cron/cron.go | 19 +++--- 4 files changed, 116 insertions(+), 66 deletions(-) diff --git a/models/cron/cron.go b/models/cron/cron.go index 348f5ce53b9..cbf980c0502 100644 --- a/models/cron/cron.go +++ b/models/cron/cron.go @@ -5,29 +5,48 @@ package cron import ( + "time" + "github.com/gogits/gogs/models" "github.com/gogits/gogs/modules/cron" + "github.com/gogits/gogs/modules/log" "github.com/gogits/gogs/modules/setting" ) var c = cron.New() func NewCronContext() { + var ( + entry *cron.Entry + err error + ) if setting.Cron.UpdateMirror.Enabled { - c.AddFunc("Update mirrors", setting.Cron.UpdateMirror.Schedule, models.MirrorUpdate) + entry, err = c.AddFunc("Update mirrors", setting.Cron.UpdateMirror.Schedule, models.MirrorUpdate) + if err != nil { + log.Fatal(4, "Cron[Update mirrors]: %v", err) + } if setting.Cron.UpdateMirror.RunAtStart { + entry.Prev = time.Now() go models.MirrorUpdate() } } if setting.Cron.RepoHealthCheck.Enabled { - c.AddFunc("Repository health check", setting.Cron.RepoHealthCheck.Schedule, models.GitFsck) + entry, err = c.AddFunc("Repository health check", setting.Cron.RepoHealthCheck.Schedule, models.GitFsck) + if err != nil { + log.Fatal(4, "Cron[Repository health check]: %v", err) + } if setting.Cron.RepoHealthCheck.RunAtStart { + entry.Prev = time.Now() go models.GitFsck() } } if setting.Cron.CheckRepoStats.Enabled { - c.AddFunc("Check repository statistics", setting.Cron.CheckRepoStats.Schedule, models.CheckRepoStats) + entry, err = c.AddFunc("Check repository statistics", setting.Cron.CheckRepoStats.Schedule, models.CheckRepoStats) + if err != nil { + log.Fatal(4, "Cron[Check repository statistics]: %v", err) + } if setting.Cron.CheckRepoStats.RunAtStart { + entry.Prev = time.Now() go models.CheckRepoStats() } } diff --git a/models/repo.go b/models/repo.go index c4304d9bee4..e4122966d59 100644 --- a/models/repo.go +++ b/models/repo.go @@ -173,7 +173,7 @@ func (repo *Repository) getOwner(e Engine) (err error) { return err } -func (repo *Repository) GetOwner() (err error) { +func (repo *Repository) GetOwner() error { return repo.getOwner(x) } @@ -326,12 +326,23 @@ func IsUsableName(name string) error { type Mirror struct { ID int64 `xorm:"pk autoincr"` RepoID int64 - RepoName string // / - Interval int // Hour. - Updated time.Time `xorm:"UPDATED"` + Repo *Repository `xorm:"-"` + Interval int // Hour. + Updated time.Time `xorm:"UPDATED"` NextUpdate time.Time } +func (m *Mirror) AfterSet(colName string, _ xorm.Cell) { + var err error + switch colName { + case "repo_id": + m.Repo, err = GetRepositoryByID(m.RepoID) + if err != nil { + log.Error(3, "GetRepositoryByID[%d]: %v", m.ID, err) + } + } +} + func getMirror(e Engine, repoId int64) (*Mirror, error) { m := &Mirror{RepoID: repoId} has, err := e.Get(m) @@ -368,7 +379,6 @@ func MirrorRepository(repoId int64, userName, repoName, repoPath, url string) er if _, err = x.InsertOne(&Mirror{ RepoID: repoId, - RepoName: strings.ToLower(userName + "/" + repoName), Interval: 24, NextUpdate: time.Now().Add(24 * time.Hour), }); err != nil { @@ -784,18 +794,6 @@ func TransferOwnership(u *User, newOwnerName string, repo *Repository) error { return fmt.Errorf("transferRepoAction: %v", err) } - // Update mirror information. - if repo.IsMirror { - mirror, err := getMirror(sess, repo.ID) - if err != nil { - return fmt.Errorf("getMirror: %v", err) - } - mirror.RepoName = newOwner.LowerName + "/" + repo.LowerName - if err = updateMirror(sess, mirror); err != nil { - return fmt.Errorf("updateMirror: %v", err) - } - } - // Change repository directory name. if err = os.Rename(RepoPath(owner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil { return fmt.Errorf("rename directory: %v", err) @@ -1122,22 +1120,32 @@ func MirrorUpdate() { isMirrorUpdating = true defer func() { isMirrorUpdating = false }() - mirrors := make([]*Mirror, 0, 10) + log.Trace("Doing: MirrorUpdate") + mirrors := make([]*Mirror, 0, 10) if err := x.Iterate(new(Mirror), func(idx int, bean interface{}) error { m := bean.(*Mirror) if m.NextUpdate.After(time.Now()) { return nil } - repoPath := filepath.Join(setting.RepoRootPath, m.RepoName+".git") + if m.Repo == nil { + log.Error(4, "Disconnected mirror repository found: %d", m.ID) + return nil + } + + repoPath, err := m.Repo.RepoPath() + if err != nil { + return fmt.Errorf("Repo.RepoPath: %v", err) + } + if _, stderr, err := process.ExecDir(10*time.Minute, repoPath, fmt.Sprintf("MirrorUpdate: %s", repoPath), "git", "remote", "update", "--prune"); err != nil { desc := fmt.Sprintf("Fail to update mirror repository(%s): %s", repoPath, stderr) log.Error(4, desc) if err = CreateRepositoryNotice(desc); err != nil { - log.Error(4, "Fail to add notice: %v", err) + log.Error(4, "CreateRepositoryNotice: %v", err) } return nil } @@ -1151,7 +1159,7 @@ func MirrorUpdate() { for i := range mirrors { if err := UpdateMirror(mirrors[i]); err != nil { - log.Error(4, "UpdateMirror", fmt.Sprintf("%s: %v", mirrors[i].RepoName, err)) + log.Error(4, "UpdateMirror[%d]: %v", mirrors[i].ID, err) } } } @@ -1164,26 +1172,28 @@ func GitFsck() { isGitFscking = true defer func() { isGitFscking = false }() + log.Trace("Doing: GitFsck") + args := append([]string{"fsck"}, setting.Cron.RepoHealthCheck.Args...) - if err := x.Where("id > 0").Iterate(new(Repository), + if err := x.Where("id>0").Iterate(new(Repository), func(idx int, bean interface{}) error { repo := bean.(*Repository) - if err := repo.GetOwner(); err != nil { - return err + repoPath, err := repo.RepoPath() + if err != nil { + return fmt.Errorf("RepoPath: %v", err) } - repoPath := RepoPath(repo.Owner.Name, repo.Name) - _, _, err := process.ExecDir(-1, repoPath, "Repository health check", "git", args...) + _, _, err = process.ExecDir(-1, repoPath, "Repository health check", "git", args...) if err != nil { desc := fmt.Sprintf("Fail to health check repository(%s)", repoPath) log.Warn(desc) if err = CreateRepositoryNotice(desc); err != nil { - log.Error(4, "Fail to add notice: %v", err) + log.Error(4, "CreateRepositoryNotice: %v", err) } } return nil }); err != nil { - log.Error(4, "repo.Fsck: %v", err) + log.Error(4, "GitFsck: %v", err) } } @@ -1210,33 +1220,55 @@ func CheckRepoStats() { isCheckingRepos = true defer func() { isCheckingRepos = false }() - // Check count watchers - results_watch, err := x.Query("SELECT r.id FROM `repository` r WHERE r.num_watches!=(SELECT count(*) FROM `watch` WHERE repo_id=r.id)") - if err != nil { - log.Error(4, "select repository check 'watch': %v", err) - } - for _, repo_id := range results_watch { - log.Trace("updating repository count 'watch'") - repoID := com.StrTo(repo_id["id"]).MustInt64() - _, err := x.Exec("UPDATE `repository` SET num_watches=(SELECT count(*) FROM `watch` WHERE repo_id=?) WHERE id=?", repoID, repoID) - if err != nil { - log.Error(4, "update repository check 'watch', repo %v: %v", repo_id, err) - } - } + log.Trace("Doing: CheckRepoStats") - // Check count stars - results_star, err := x.Query("SELECT s.id FROM `repository` s WHERE s.num_stars!=(SELECT count(*) FROM `star` WHERE repo_id=s.id)") + // ***** START: Watch ***** + results, err := x.Query("SELECT repo.id FROM `repository` repo WHERE repo.num_watches!=(SELECT COUNT(*) FROM `watch` WHERE repo_id=repo.id)") if err != nil { - log.Error(4, "select repository check 'star': %v", err) + log.Error(4, "Select repository check 'watch': %v", err) + return } - for _, repo_id := range results_star { - log.Trace("updating repository count 'star'") - repoID := com.StrTo(repo_id["id"]).MustInt64() - _, err := x.Exec("UPDATE `repository` SET .num_stars=(SELECT count(*) FROM `star` WHERE repo_id=?) WHERE id=?", repoID, repoID) + for _, watch := range results { + repoID := com.StrTo(watch["id"]).MustInt64() + log.Trace("Updating repository count 'watch': %d", repoID) + _, err = x.Exec("UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=?) WHERE id=?", repoID, repoID) if err != nil { - log.Error(4, "update repository check 'star', repo %v: %v", repo_id, err) + log.Error(4, "Update repository check 'watch'[%d]: %v", repoID, err) } } + // ***** END: Watch ***** + + // ***** START: Star ***** + results, err = x.Query("SELECT repo.id FROM `repository` repo WHERE repo.num_stars!=(SELECT COUNT(*) FROM `star` WHERE repo_id=repo.id)") + if err != nil { + log.Error(4, "Select repository check 'star': %v", err) + return + } + for _, star := range results { + repoID := com.StrTo(star["id"]).MustInt64() + log.Trace("Updating repository count 'star': %d", repoID) + _, err = x.Exec("UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", repoID, repoID) + if err != nil { + log.Error(4, "Update repository check 'star'[%d]: %v", repoID, err) + } + } + // ***** END: Star ***** + + // ***** START: Label ***** + results, err = x.Query("SELECT label.id FROM `label` WHERE label.num_issues!=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=label.id)") + if err != nil { + log.Error(4, "Select label check 'num_issues': %v", err) + return + } + for _, label := range results { + labelID := com.StrTo(label["id"]).MustInt64() + log.Trace("Updating label count 'num_issues': %d", labelID) + _, err = x.Exec("UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", labelID, labelID) + if err != nil { + log.Error(4, "Update label check 'num_issues'[%d]: %v", labelID, err) + } + } + // ***** END: Label ***** } // _________ .__ .__ ___. __ .__ diff --git a/modules/auth/ldap/ldap.go b/modules/auth/ldap/ldap.go index 50101349dbc..82d66fec374 100644 --- a/modules/auth/ldap/ldap.go +++ b/modules/auth/ldap/ldap.go @@ -42,7 +42,7 @@ func (ls Ldapsource) FindUserDN(name string) (string, bool) { if ls.BindDN != "" && ls.BindPassword != "" { err = l.Bind(ls.BindDN, ls.BindPassword) if err != nil { - log.Debug("Failed to bind as BindDN: %s, %s", ls.BindDN, err.Error()) + log.Debug("Failed to bind as BindDN[%s]: %v", ls.BindDN, err) return "", false } log.Trace("Bound as BindDN %s", ls.BindDN) @@ -60,7 +60,7 @@ func (ls Ldapsource) FindUserDN(name string) (string, bool) { // Ensure we found a user sr, err := l.Search(search) if err != nil || len(sr.Entries) < 1 { - log.Debug("Failed search using filter %s: %s", userFilter, err.Error()) + log.Debug("Failed search using filter[%s]: %v", userFilter, err) return "", false } else if len(sr.Entries) > 1 { log.Debug("Filter '%s' returned more than one user.", userFilter) @@ -95,7 +95,7 @@ func (ls Ldapsource) SearchEntry(name, passwd string) (string, string, string, b log.Trace("Binding with userDN: %s", userDN) err = l.Bind(userDN, passwd) if err != nil { - log.Debug("LDAP auth. failed for %s, reason: %s", userDN, err.Error()) + log.Debug("LDAP auth. failed for %s, reason: %v", userDN, err) return "", "", "", false } @@ -108,7 +108,7 @@ func (ls Ldapsource) SearchEntry(name, passwd string) (string, string, string, b sr, err := l.Search(search) if err != nil { - log.Error(4, "LDAP Search failed unexpectedly! (%s)", err.Error()) + log.Error(4, "LDAP Search failed unexpectedly! (%v)", err) return "", "", "", false } else if len(sr.Entries) < 1 { log.Error(4, "LDAP Search failed unexpectedly! (0 entries)") diff --git a/modules/cron/cron.go b/modules/cron/cron.go index dbf0174b869..3491afac99c 100644 --- a/modules/cron/cron.go +++ b/modules/cron/cron.go @@ -91,34 +91,33 @@ type FuncJob func() func (f FuncJob) Run() { f() } // AddFunc adds a func to the Cron to be run on the given schedule. -func (c *Cron) AddFunc(desc, spec string, cmd func()) error { +func (c *Cron) AddFunc(desc, spec string, cmd func()) (*Entry, error) { return c.AddJob(desc, spec, FuncJob(cmd)) } // AddFunc adds a Job to the Cron to be run on the given schedule. -func (c *Cron) AddJob(desc, spec string, cmd Job) error { +func (c *Cron) AddJob(desc, spec string, cmd Job) (*Entry, error) { schedule, err := Parse(spec) if err != nil { - return err + return nil, err } - c.Schedule(desc, spec, schedule, cmd) - return nil + return c.Schedule(desc, spec, schedule, cmd), nil } // Schedule adds a Job to the Cron to be run on the given schedule. -func (c *Cron) Schedule(desc, spec string, schedule Schedule, cmd Job) { +func (c *Cron) Schedule(desc, spec string, schedule Schedule, cmd Job) *Entry { entry := &Entry{ Description: desc, Spec: spec, Schedule: schedule, Job: cmd, } - if !c.running { + if c.running { + c.add <- entry + } else { c.entries = append(c.entries, entry) - return } - - c.add <- entry + return entry } // Entries returns a snapshot of the cron entries.