test-results: package frontend: add upload handler

go/src/infra/appengine/test-results/frontend/upload.go

+package frontend
+
+import (
+        "bytes"
+        "encoding/json"
+        "errors"
+        "io"
+        "mime/multipart"
+        "net/http"
+        "strconv"
+        "strings"
+        "sync"
+
+        "golang.org/x/net/context"
+
+        "github.com/luci/gae/service/datastore"
+        "github.com/luci/luci-go/server/router"
+
+        "infra/appengine/test-results/model"
+)
+
+type statusError struct {
+        error
+        code int
+}
+
+func (se *statusError) StatusCode() int {
+        return se.code
+}
+
+// MarshalJSON marshals status error to JSON.
+func (se *statusError) MarshalJSON() ([]byte, error) {
+        m := map[string]interface{}{}
+
+        if se == nil || se.error == nil {
+                return json.Marshal(m)
+        }
+
+        m["error"] = se.Error()
+        m["status"] = se.code
+        return json.Marshal(m)
+}
+
+// UploadParams is the multipart form values in a
+// TestFile upload request.
+type UploadParams struct {
+        Master, DeprMaster, Builder, TestType string

[estaab, 2016/08/12 15:29:07]

What's DeprMaster? perhaps document since it's not obvious

[nishanths, 2016/08/16 05:16:29]

Done.

+}
+
+type contextKey int
+
+const uploadContextKey = contextKey(0)
+
+// GetUploadParams returns the UploadParams from the context if
+// present or nil otherwise.
+func GetUploadParams(c context.Context) *UploadParams {
+        if v := c.Value(uploadContextKey); v != nil {
+                return v.(*UploadParams)
+        }
+        return nil
+}
+
+// SetUploadParams returns a new context with the supplied
+// UploadParams added to it.
+func SetUploadParams(c context.Context, p *UploadParams) context.Context {
+        return context.WithValue(c, uploadContextKey, p)
+}
+
+func cleanTestType(test string) string {

[estaab, 2016/08/12 15:29:07]

isn't this also in builders.go? Would there be a way to not duplicate it? If
not, can you reference the other file and mention we need to keep them in sync?

[nishanths, 2016/08/16 05:16:29]

Done. Removed from here. Both instances are in the same package.
(duplicated it here because I was unsure which CL would land first)

+        patched := false
+        if strings.Contains(test, " (with patch)") {
+                patched = true
+                test = strings.Replace(test, " (with patch)", "", 1)
+        }
+        if strings.HasPrefix(test, "Instrumentation test ") {
+                test = test[len("Instrumentation test "):]
+        }
+        if i := strings.Index(test, " "); i != -1 {
+                test = test[:i]
+        }
+        if !patched {
+                return test
+        }
+        return test + " (with patch)"
+}
+
+// withParsedUploadForm is middleware that verifies and adds
+// multipart form upload data to the context.
+//
+// If there is an error parsing the form or required
+// values are missing, WithParsed writes the HTTP error
+// to the response writer and stops execution of the request.
+func withParsedUploadForm(ctx *router.Context, next router.Handler) {
+        w, r := ctx.Writer, ctx.Request
+        const _1M = 1 << 20
+
+        if err := r.ParseMultipartForm(_1M); err != nil {
+                http.Error(w, err.Error(), http.StatusInternalServerError)
+                return
+        }
+
+        u := &UploadParams{}
+
+        if v := r.MultipartForm.Value["master"]; len(v) > 0 {
+                if m := model.MasterByName(v[0]); m != nil {
+                        u.Master = m.Identifier
+                        u.DeprMaster = v[0]
+                } else {
+                        u.Master = v[0]
+                }
+        }
+
+        if v := r.MultipartForm.Value["builder"]; len(v) > 0 {
+                u.Builder = v[0]
+        } else {
+                http.Error(w, "missing builder", http.StatusBadRequest)
+                return
+        }
+
+        if v := r.MultipartForm.Value["testtype"]; len(v) > 0 {
+                u.TestType = cleanTestType(v[0])
+        }
+
+        if _, ok := r.MultipartForm.File["file"]; !ok {
+                http.Error(w, "missing file", http.StatusBadRequest)
+                return
+        }
+
+        ctx.Context = SetUploadParams(ctx.Context, u)
+        next(ctx)
+}
+
+// uploadHandler is the HTTP handler for upload
+// requests.
+func uploadHandler(ctx *router.Context) {
+        c, w, r := ctx.Context, ctx.Writer, ctx.Request
+        fileheaders := r.MultipartForm.File["file"]
+
+        for _, fh := range fileheaders {
+                if err := doFileUpload(c, fh); err != nil {
+                        code := http.StatusInternalServerError
+                        if se, ok := err.(statusError); ok {
+                                code = se.code
+                        }
+                        http.Error(w, err.Error(), code)
+                        return
+                }
+        }
+
+        io.WriteString(w, "OK")
+}
+
+func doFileUpload(c context.Context, fh *multipart.FileHeader) error {
+        file, err := fh.Open()
+        if err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+        defer file.Close()
+
+        switch fh.Filename {
+        case "incremental_results.json":
+                var incr model.AggregateResult
+                if err := json.NewDecoder(file).Decode(&incr); err != nil {
+                        return statusError{err, http.StatusBadRequest}
+                }
+                return updateIncremental(c, &incr)
+        case "full_results.json":
+                return updateFullResults(c, file)
+        default:
+                return uploadTestFile(c, file, fh.Filename)
+        }
+}
+
+// uploadTestFile creates a new TestFile from the values in context
+// and supplied data, and puts it to the datastore.
+func uploadTestFile(c context.Context, data io.Reader, filename string) error {
+        var buf bytes.Buffer
+        tee := io.TeeReader(data, &buf)
+
+        aux := struct {
+                N string `json:"build_number,omitempty"`
+        }{}
+
+        dec := json.NewDecoder(tee)
+        if err := dec.Decode(&aux); err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+
+        bn := 0
+
+        if aux.N != "" {
+                n, err := strconv.Atoi(aux.N)
+                if err != nil {
+                        return statusError{errors.New("invalid build_number"), http.StatusBadRequest}
+                }
+                bn = n
+        }
+
+        p := GetUploadParams(c)
+        tf := model.TestFile{
+                Master:      p.Master,
+                Builder:     p.Builder,
+                TestType:    p.TestType,
+                BuildNumber: model.BuildNum(bn),
+                Name:        filename,
+                Data:        io.MultiReader(&buf, dec.Buffered()),
+        }
+        if err := tf.PutData(c); err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+        return nil
+}
+
+// updateFullResults puts the supplied data as "full_results.json"
+// to the datastore, and updates corresponding "results.json" and
+// "results-small.json" files in the datastore.
+//
+// The supplied data should unmarshal into model.FullResults.
+// Otherwise, an error is returned.
+func updateFullResults(c context.Context, data io.Reader) error {
+        buf := &bytes.Buffer{}
+        tee := io.TeeReader(data, buf)
+        dec := json.NewDecoder(tee)
+
+        var f model.FullResult
+        if err := dec.Decode(&f); err != nil {
+                return statusError{err, http.StatusBadRequest}
+        }
+
+        var wg sync.WaitGroup
+        errCh := make(chan error, 2)
+
+        wg.Add(1)
+        go func() {
+                defer wg.Done()
+                errCh <- uploadTestFile(
+                        c, io.MultiReader(buf, dec.Buffered()), "full_results.json",
+                )
+        }()
+
+        wg.Add(1)
+        go func() {
+                defer wg.Done()
+                incr, err := f.AggregateResult()
+                if err != nil {
+                        errCh <- statusError{err, http.StatusBadRequest}
+                        return
+                }
+                errCh <- updateIncremental(c, &incr)
+        }()
+
+        wg.Wait()
+        close(errCh)
+        for err := range errCh {
+                if err != nil {
+                        return err
+                }
+        }
+        return nil
+}
+
+// updateIncremental gets "results.json" and "results-small.json"
+// for values in context, merges incr into them, and puts the updated
+// files to the datastore.
+func updateIncremental(c context.Context, incr *model.AggregateResult) error {
+        u := GetUploadParams(c)
+        p := model.TestFileParams{
+                Master:   u.Master,
+                Builder:  u.Builder,
+                TestType: u.TestType,
+        }
+
+        names := []string{"results.json", "results-small.json"}
+        files := make([]struct {
+                tf   *model.TestFile
+                aggr *model.AggregateResult
+                err  error
+        }, len(names))
+
+        var wg sync.WaitGroup
+
+        for i, name := range names {
+                i, name := i, name
+                wg.Add(1)
+                go func() {
+                        defer wg.Done()
+                        tf, err := getTestFileAlt(c, p, u.DeprMaster)
+                        if err != nil {
+                                if _, ok := err.(ErrNoMatches); ok {
+                                        files[i].tf = &model.TestFile{
+                                                Master:      p.Master,
+                                                Builder:     p.Builder,
+                                                TestType:    p.TestType,
+                                                BuildNumber: -1,
+                                                Name:        name,
+                                        }
+                                } else {
+                                        files[i].err = err
+                                }
+                                return
+                        }
+                        files[i].tf = tf
+                        if err := tf.GetData(c); err != nil {
+                                files[i].err = err
+                                return
+                        }
+                        if err := json.NewDecoder(tf.Data).Decode(files[i].aggr); err != nil {
+                                files[i].err = err
+                                return
+                        }
+                }()
+        }
+
+        wg.Wait()
+        for _, file := range files {
+                if file.err != nil {
+                        return file.err
+                }
+        }
+
+        errs := make([]error, len(files))
+
+        return datastore.Get(c).RunInTransaction(func(c context.Context) error {
+                for i, file := range files {
+                        i, file := i, file
+                        wg.Add(1)

[estaab, 2016/08/12 15:29:07]

It seems a little messy to reuse the WaitGroup from the outer scope since the
reader has to double check what state it's in to understand this function. Maybe
just make a new one?

[nishanths, 2016/08/16 05:16:29]

Done.

+                        go func() {
+                                defer wg.Done()
+                                errs[i] = updateAggregate(c, file.tf, file.aggr, incr)
+                        }()
+                }
+
+                wg.Wait()
+                // Prioritize returning http.StatusInternalServerError status
+                // code errors over other errors.
+                var e error
+                for _, err := range errs {
+                        se, ok := err.(statusError)
+                        if ok && se.code == http.StatusInternalServerError {
+                                return se
+                        }
+                        e = err
+                }
+                return e
+        }, &datastore.TransactionOptions{XG: true})
+}
+
+// getTestFileAlt returns the the first TestFile in the datastore for
+// the query formed by calling p.Query().
+//
+// The function tries to find the first TestFile using p. If no such TestFile
+// exists the function sets p.Master to altMaster and tries again.
+// If altMaster is empty, the function does not perform the additional try.
+func getTestFileAlt(c context.Context, p model.TestFileParams, altMaster string) (ret *model.TestFile, err error) {
+        a, err := getFirstTestFile(c, p.Query())
+        if err == nil {
+                return a, nil
+        }
+        if _, ok := err.(ErrNoMatches); ok && altMaster == "" {
+                return nil, err
+        }
+
+        origMaster := p.Master
+        p.Master = altMaster
+
+        a, err = getFirstTestFile(c, p.Query())
+        if err == nil {
+                a.Master = origMaster
+                return a, nil
+        }
+
+        return nil, err
+}
+
+// updateAggregate updates tf with the result of merging incr into
+// aggr.
+func updateAggregate(c context.Context, tf *model.TestFile, aggr, incr *model.AggregateResult) error {
+        if !model.IsAggregateTestFile(tf.Name) {
+                return errors.New("frontend: tf should be an aggregate test file")
+        }
+
+        size := model.ResultsSize
+        if tf.Name == "results-small.json" {
+                size = model.ResultsSmallSize
+        }
+
+        if aggr == nil {
+                aggr = incr
+        } else {
+                if err := aggr.Merge(incr); err != nil {
+                        switch err {
+                        case model.ErrBuilderNameConflict:
+                                return statusError{err, http.StatusBadRequest}
+                        case model.ErrBuildNumberConflict:
+                                return statusError{err, http.StatusConflict}
+                        default:
+                                return statusError{err, http.StatusInternalServerError}
+                        }
+                }
+        }
+
+        if err := aggr.Trim(size); err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+
+        b := &bytes.Buffer{}
+        if err := json.NewEncoder(b).Encode(&aggr); err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+
+        tf.Data = b
+        if err := tf.PutData(c); err != nil {
+                return statusError{err, http.StatusInternalServerError}
+        }
+
+        return nil
+}