dashboard: ensure that we ever store valid commits

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews, rsc
https://golang.org/cl/160960043

dashboard/app/build/build.go

@@ -127,6 +127,19 @@ func (c *Commit) Valid() error {
 	return nil
 }
 
+func putCommit(c appengine.Context, com *Commit) error {
+	if err := com.Valid(); err != nil {
+		return fmt.Errorf("putting Commit: %v", err)
+	}
+	if com.Num == 0 {
+		return fmt.Errorf("putting Commit: com.Num == 0")
+	}
+	if _, err := datastore.Put(c, com.Key(c), com); err != nil {
+		return fmt.Errorf("putting Commit: %v", err)
+	}
+	return nil
+}
+
 // each result line is approx 105 bytes. This constant is a tradeoff between
 // build history and the AppEngine datastore limit of 1mb.
 const maxResults = 1000
@@ -150,10 +163,7 @@ func (com *Commit) AddResult(c appengine.Context, r *Result) error {
 		// otherwise, add the new result data for this builder.
 		com.ResultData = trim(append(com.ResultData, r.Data()), maxResults)
 	}
-	if _, err := datastore.Put(c, com.Key(c), com); err != nil {
+	return putCommit(c, com)
-		return fmt.Errorf("putting Commit: %v", err)
-	}
-	return nil
 }
 
 // AddPerfResult remembers that the builder has run the benchmark on the commit.
@@ -172,10 +182,7 @@ func (com *Commit) AddPerfResult(c appengine.Context, builder, benchmark string)
 		}
 	}
 	com.PerfResults = append(com.PerfResults, s)
-	if _, err := datastore.Put(c, com.Key(c), com); err != nil {
+	return putCommit(c, com)
-		return fmt.Errorf("putting Commit: %v", err)
-	}
-	return nil
 }
 
 func trim(s []string, n int) []string {

dashboard/app/build/handler.go

@@ -192,8 +192,8 @@ func addCommit(c appengine.Context, com *Commit) error {
 		}
 	}
 	// put the Commit
-	if _, err = datastore.Put(c, com.Key(c), com); err != nil {
+	if err = putCommit(c, com); err != nil {
-		return fmt.Errorf("putting Commit: %v", err)
+		return err
 	}
 	if com.NeedsBenchmarking {
 		// add to CommitRun

dashboard/app/build/notify.go

@@ -230,8 +230,7 @@ func commonNotify(c appengine.Context, com *Commit, builder, logHash string) err
 	c.Infof("%s is broken commit; notifying", com.Hash)
 	notifyLater.Call(c, com, builder, logHash) // add task to queue
 	com.FailNotificationSent = true
-	_, err := datastore.Put(c, com.Key(c), com)
+	return putCommit(c, com)
-	return err
 }
 
 // sendFailMail sends a mail notification that the build failed on the

dashboard/app/build/update.go

@@ -58,8 +58,8 @@ func updateBenchmark(w http.ResponseWriter, r *http.Request) {
 				continue
 			}
 			com.NeedsBenchmarking = true
-			if _, err := datastore.Put(c, com.Key(c), com); err != nil {
+			if err := putCommit(c, com); err != nil {
-				return fmt.Errorf("putting Commit: %v", err)
+				return err
 			}
 			ncommit++