Milo buildbot builder page: Add pagnation with cursors.

milo/appengine/buildbot/builder.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
+        "crypto/sha1"
+        "encoding/base64"
         "errors"
         "fmt"
         "sort"
         "strings"
         "time"
 
         "github.com/luci/gae/service/datastore"
+        "github.com/luci/gae/service/memcache"
 
         "github.com/luci/luci-go/common/clock"
         "github.com/luci/luci-go/common/logging"
         "github.com/luci/luci-go/milo/api/resp"
         "golang.org/x/net/context"
 )
 
 // builderRef is used for keying specific builds in a master json.
 type builderRef struct {
         builder  string
@@ skipping 53 matching lines @@
                 },
                 Text:     mergeText(b.Text),
                 Blame:    blame(b),
                 Revision: b.Sourcestamp.Revision,
         }
 }
 
 // getBuilds fetches all of the recent builds from the .  Note that
 // getBuilds() does not perform ACL checks.
 func getBuilds(
-        c context.Context, masterName, builderName string, finished bool, limit int) (
-        []*resp.BuildSummary, error) {
+        c context.Context, masterName, builderName string, finished bool, limit int, cursor *datastore.Cursor) (
+        []*resp.BuildSummary, *datastore.Cursor, error) {
 
         // TODO(hinoka): Builder specific structs.
         result := []*resp.BuildSummary{}
         q := datastore.NewQuery("buildbotBuild")
         q = q.Eq("finished", finished)
         q = q.Eq("master", masterName)
         q = q.Eq("builder", builderName)
-        if limit != 0 {
-                q = q.Limit(int32(limit))
+        q = q.Order("-number")
+        if cursor != nil {
+                q = q.Start(*cursor)
         }
-        q = q.Order("-number")
-        buildbots := []*buildbotBuild{}
-        err := getBuildQueryBatcher(c).GetAll(c, q, &buildbots)
+        buildbots, nextCursor, err := runBuildsQuery(c, q, int32(limit))
         if err != nil {
-                return nil, err
+                return nil, nil, err
         }
         for _, b := range buildbots {
                 result = append(result, getBuildSummary(b))
         }
-        return result, nil
+        return result, nextCursor, nil
+}
+
+// maybeSetGetCursor is a cheesy way to implement bidirectional paging with forward-only
+// datastore cursor by creating a mapping of nextCursor -> thisCursor
+// in memcache.  maybeSetGetCursor stores the future mapping, then returns prevCursor
+// in the mapping for thisCursor -> prevCursor, if available.
+func maybeSetGetCursor(c context.Context, thisCursor, nextCursor *datastore.Cursor) (*datastore.Cursor, bool) {
+        key := func(c datastore.Cursor) string {
+                // Memcache key limit is 250 bytes, hash our cursor to get under this limit.
+                blob := sha1.Sum([]byte(c.String()))
+                return "v2:cursors:builders:" + base64.StdEncoding.EncodeToString(blob[:])

[nodir, 2017/04/18 20:27:49]

are you sure cursors for buildbucket builders will be different? maybe
s/builders/buildbot_builders/ or something

[nodir, 2017/04/18 20:27:49]

limit has to be included in the cache key. Bug repro:

Go to
https://1680-8fa48d3-tainted-hinoka-dot-luci-milo-dev.appspot.com/buildbot/ch...
I see #23464-#23563. Correct.
Click next
I see #23364-#23463. Correct.
Set page=25
I see #23439-#23463. Correct
Click prev
I see #23539-#23563. Incorrect. Expected: #23464-#23489

[hinoka, 2017/04/20 21:37:08]

Done.

[hinoka, 2017/04/20 21:37:08]

Blarg.... basically you can't actually go back once you click next -> switch
limit.... I don't think theres a fix for this.  Will just remove prev entirely
in this case and hope no one notices.

+        }
+        // Set the next cursor to this cursor mapping, if available.
+        if nextCursor != nil {
+                thisKey := key(*nextCursor)

[nodir, 2017/04/18 20:27:49]

nextKey? it is used only once, consider inlining it, consistent with L141

[hinoka, 2017/04/20 21:37:08]

Done.

+                item := memcache.NewItem(c, thisKey)
+                if thisCursor == nil {
+                        // Make sure we know it exists, just empty
+                        item.SetValue([]byte{})
+                } else {
+                        item.SetValue([]byte((*thisCursor).String()))
+                }
+                item.SetExpiration(24 * time.Hour)
+                memcache.Set(c, item)
+        }
+        // Try to get the last cursor, if valid and available.
+        if thisCursor == nil {
+                return nil, false
+        }
+        if item, err := memcache.GetKey(c, key(*thisCursor)); err == nil {
+                if len(item.Value()) == 0 {
+                        return nil, true
+                }
+                if prevCursor, err := datastore.DecodeCursor(c, string(item.Value())); err == nil {
+                        return &prevCursor, true
+                }
+        }
+        return nil, false
 }
 
 var errMasterNotFound = errors.New(
         "Either the request resource was not found or you have insufficient permissions")
 var errNotAuth = errors.New("You are not authenticated, try logging in")
 
 // builderImpl is the implementation for getting a milo builder page from buildbot.
 // This gets:
 // * Current Builds from querying the master json from the datastore.
 // * Recent Builds from a cron job that backfills the recent builds.
-func builderImpl(c context.Context, masterName, builderName string, limit int) (*resp.Builder, error) {
+func builderImpl(
+        c context.Context, masterName, builderName string, limit int,
+        cursor *datastore.Cursor) (*resp.Builder, error) {

[nodir, 2017/04/18 20:27:49]

consider making cursor a string here. datastore is implementation detail of this
function and so far this function did not expose it, but now it does

this would make http.go return HTTP 500 for a bad cursor, but looks like we
already have this problem: if a builder name is misspelled, we return HTTP 500.
This should be fixed (in a separate CL)

[hinoka, 2017/04/20 21:37:08]

Done.

+
         result := &resp.Builder{
                 Name: builderName,
         }
         master, t, err := getMasterJSON(c, masterName)
         if err != nil {
                 return nil, err
         }
         if clock.Now(c).Sub(t) > 2*time.Minute {
                 warning := fmt.Sprintf(
                         "WARNING: Master data is stale (last updated %s)", t)
@@ skipping 29 matching lines @@
                 result.PendingBuilds[i].Blame = make([]*resp.Commit, len(pb.Source.Changes))
                 for j, cm := range pb.Source.Changes {
                         result.PendingBuilds[i].Blame[j] = &resp.Commit{
                                 AuthorEmail: cm.Who,
                                 CommitURL:   cm.Revlink,
                         }
                 }
         }
 
         // This is CPU bound anyways, so there's no need to do this in parallel.
-        finishedBuilds, err := getBuilds(c, masterName, builderName, true, limit)
+        finishedBuilds, nextCursor, err := getBuilds(c, masterName, builderName, true, limit, cursor)
         if err != nil {
                 return nil, err
         }
-        currentBuilds, err := getBuilds(c, masterName, builderName, false, 0)
+        if prevCursor, ok := maybeSetGetCursor(c, cursor, nextCursor); ok {
+                if prevCursor == nil {
+                        // Magic string to signal display prev without cursor
+                        result.PrevCursor = "EMPTY"
+                } else {
+                        result.PrevCursor = (*prevCursor).String()
+                }
+        }
+        if nextCursor != nil {
+                result.NextCursor = (*nextCursor).String()
+        }
+        // Cursor is not needed for current builds.
+        currentBuilds, _, err := getBuilds(c, masterName, builderName, false, 0, nil)
         if err != nil {
                 return nil, err
         }
         // currentBuilds is presented in reversed order, so flip it
         for i, j := 0, len(currentBuilds)-1; i < j; i, j = i+1, j-1 {
                 currentBuilds[i], currentBuilds[j] = currentBuilds[j], currentBuilds[i]
         }
         result.CurrentBuilds = currentBuilds
 
         for _, fb := range finishedBuilds {
                 if fb != nil {
                         result.FinishedBuilds = append(result.FinishedBuilds, fb)
                 }
         }
         return result, nil
 }