From e8b993e7dacf9c5fd018ed41f24ae9344e587be7 Mon Sep 17 00:00:00 2001 From: Michael Nutt Date: Tue, 30 Dec 2014 11:37:57 -0500 Subject: [PATCH] Add support for after_success and after_failure to email notifications This allows you to restrict email notifications to only be sent after the build changes from success to failure or failure to success. It errs on the side of sending the notification; if the build is in another state (hung, for instance) or there was no previous build on the branch the email will also be sent. Since the notify plugin shouldn't really have any responsibility for querying the database to find the previous commit's status, we store it on the commit when we save it. --- plugin/notify/email/email.go | 28 ++++++++- server/datastore/database/commit.go | 33 ++++++++-- server/datastore/database/commit_test.go | 78 ++++++++++++++++++++++++ server/datastore/database/database.go | 1 + server/datastore/migrate/migrate.go | 10 +++ shared/model/commit.go | 1 + 6 files changed, 143 insertions(+), 8 deletions(-) diff --git a/plugin/notify/email/email.go b/plugin/notify/email/email.go index f113101f5..6d38ff13a 100644 --- a/plugin/notify/email/email.go +++ b/plugin/notify/email/email.go @@ -12,9 +12,11 @@ import ( ) const ( - NotifyAlways = "always" // always send email notification - NotifyNever = "never" // never send email notifications - NotifyAuthor = "author" // only send email notifications to the author + NotifyAlways = "always" // always send email notification + NotifyNever = "never" // never send email notifications + NotifyAuthor = "author" // only send email notifications to the author + NotifyAfterFailure = "after_failure" // only send a notification if the previous commit failed + NotifyAfterSuccess = "after_success" // only send a notification if the previous commit succeeded NotifyTrue = "true" // alias for NotifyTrue NotifyFalse = "false" // alias for NotifyFalse @@ -70,6 +72,16 @@ func (e *Email) sendFailure(context *model.Request) error { switch e.Failure { case NotifyFalse, NotifyNever, NotifyOff: return nil + // if the last commit in this branch was a success, notify + case NotifyAfterSuccess: + if context.Commit.PriorStatus != "Success" { + return nil + } + // if the last commit in this branch was a failure, notify + case NotifyAfterFailure: + if context.Commit.PriorStatus != "Failure" { + return nil + } // if configured to email the author, replace // the recipiends with the commit author email. case NotifyBlame, NotifyAuthor: @@ -103,6 +115,16 @@ func (e *Email) sendSuccess(context *model.Request) error { switch e.Success { case NotifyFalse, NotifyNever, NotifyOff: return nil + // if the last commit in this branch was a success, notify + case NotifyAfterSuccess: + if context.Commit.PriorStatus == "Failure" { + return nil + } + // if the last commit in this branch was a failure, notify + case NotifyAfterFailure: + if context.Commit.PriorStatus == "Success" { + return nil + } // if configured to email the author, replace // the recipiends with the commit author email. case NotifyBlame, NotifyAuthor: diff --git a/server/datastore/database/commit.go b/server/datastore/database/commit.go index ac743e69a..72f2ecc5a 100644 --- a/server/datastore/database/commit.go +++ b/server/datastore/database/commit.go @@ -40,6 +40,15 @@ func (db *Commitstore) GetCommitLast(repo *model.Repo, branch string) (*model.Co return commit, err } +// GetCommitPrior retrieves the latest commit +// from the datastore for the specified repository +// and branch. +func (db *Commitstore) GetCommitPrior(oldCommit *model.Commit) (*model.Commit, error) { + var commit = new(model.Commit) + var err = meddler.QueryRow(db, commit, rebind(commitPriorQuery), oldCommit.RepoID, oldCommit.Branch, oldCommit.Created) + return commit, err +} + // GetCommitList retrieves a list of latest commits // from the datastore for the specified repository. func (db *Commitstore) GetCommitList(repo *model.Repo) ([]*model.Commit, error) { @@ -70,16 +79,18 @@ func (db *Commitstore) PostCommit(commit *model.Commit) error { commit.Created = time.Now().UTC().Unix() } commit.Updated = time.Now().UTC().Unix() + + priorCommit, err := db.GetCommitPrior(commit) + if err == nil { + commit.PriorStatus = priorCommit.Status + } + return meddler.Save(db, commitTable, commit) } // PutCommit saves a commit in the datastore. func (db *Commitstore) PutCommit(commit *model.Commit) error { - if commit.Created == 0 { - commit.Created = time.Now().UTC().Unix() - } - commit.Updated = time.Now().UTC().Unix() - return meddler.Save(db, commitTable, commit) + return db.PostCommit(commit) } // DelCommit removes the commit from the datastore. @@ -170,6 +181,18 @@ ORDER BY commit_id DESC LIMIT 1 ` +// SQL query to retrieve the prior Commit (by commit_created) in the same branch and repo as the specified Commit. +const commitPriorQuery = ` +SELECT * +FROM commits +WHERE repo_id = ? + AND commit_branch = ? + AND commit_created < ? + AND commit_status IN ('Success', 'Failure') +ORDER BY commit_created DESC +LIMIT 1 +` + // SQL statement to cancel all running Commits. const commitKillStmt = ` UPDATE commits SET commit_status = 'Killed' diff --git a/server/datastore/database/commit_test.go b/server/datastore/database/commit_test.go index d629ee250..974d40133 100644 --- a/server/datastore/database/commit_test.go +++ b/server/datastore/database/commit_test.go @@ -48,6 +48,84 @@ func TestCommitstore(t *testing.T) { g.Assert(commit.ID != 0).IsTrue() }) + g.It("Should Get a Commit's prior status", func() { + commit := model.Commit{ + RepoID: 1, + Branch: "foo", + Status: "Failure", + Created: 1419892236, + Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", + } + err := cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "").IsTrue() + + commit = model.Commit{ + RepoID: 1, + Branch: "foo", + Status: "Success", + Created: 1419893583, + Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", + } + err = cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "Failure").IsTrue() + }) + + g.It("Should not find prior status from commits on other branches", func() { + commit := model.Commit{ + RepoID: 1, + Branch: "foo", + Status: "Failure", + Created: 1419892236, + Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", + } + err := cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "").IsTrue() + + commit = model.Commit{ + RepoID: 1, + Branch: "bar", + Status: "Success", + Created: 1419893583, + Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", + } + err = cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "").IsTrue() + }) + + g.It("Should not find prior status from commits that didn't succeed or fail", func() { + commit := model.Commit{ + RepoID: 1, + Branch: "foo", + Status: "Pending", + Created: 1419892236, + Sha: "85f8c029b902ed9400bc600bac301a0aadb144ac", + } + err := cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "").IsTrue() + + commit = model.Commit{ + RepoID: 1, + Branch: "bar", + Status: "Success", + Created: 1419893583, + Sha: "25f8c029b902ed9400bc600bac301a0aadb144ac", + } + err = cs.PostCommit(&commit) + g.Assert(err == nil).IsTrue() + g.Assert(commit.ID != 0).IsTrue() + g.Assert(commit.PriorStatus == "").IsTrue() + }) + g.It("Should Get a Commit by ID", func() { commit := model.Commit{ RepoID: 1, diff --git a/server/datastore/database/database.go b/server/datastore/database/database.go index 6ca59d66f..aecb32e9c 100644 --- a/server/datastore/database/database.go +++ b/server/datastore/database/database.go @@ -38,6 +38,7 @@ func Connect(driver, datasource string) (*sql.DB, error) { var migrations = []migration.Migrator{ migrate.Setup, migrate.Migrate_20142110, + migrate.Migrate_20143012, } return migration.Open(driver, datasource, migrations) } diff --git a/server/datastore/migrate/migrate.go b/server/datastore/migrate/migrate.go index 42a3c57c5..5985cba10 100644 --- a/server/datastore/migrate/migrate.go +++ b/server/datastore/migrate/migrate.go @@ -39,6 +39,12 @@ func Migrate_20142110(tx migration.LimitedTx) error { return nil } +// Migrate_20143012 adds the prior commit's status to the current commit +func Migrate_20143012(tx migration.LimitedTx) error { + _, err := tx.Exec(transform(commitPriorStatusColumn)) + return err +} + var userTable = ` CREATE TABLE IF NOT EXISTS users ( user_id INTEGER PRIMARY KEY AUTOINCREMENT @@ -110,6 +116,10 @@ var repoTokenUpdate = ` UPDATE repos SET repo_token = ''; ` +var commitPriorStatusColumn = ` +ALTER TABLE commits ADD COLUMN commit_prior_status VARCHAR(255); +` + var commitTable = ` CREATE TABLE IF NOT EXISTS commits ( commit_id INTEGER PRIMARY KEY AUTOINCREMENT diff --git a/shared/model/commit.go b/shared/model/commit.go index da8c0c656..0bc3aa57e 100644 --- a/shared/model/commit.go +++ b/shared/model/commit.go @@ -8,6 +8,7 @@ type Commit struct { ID int64 `meddler:"commit_id,pk" json:"id"` RepoID int64 `meddler:"repo_id" json:"-"` Status string `meddler:"commit_status" json:"status"` + PriorStatus string `meddler:"commit_prior_status" json:"prior_status"` Started int64 `meddler:"commit_started" json:"started_at"` Finished int64 `meddler:"commit_finished" json:"finished_at"` Duration int64 `meddler:"commit_duration" json:"duration"`