Milo: Fix ?limit= param

This is actually a query (form) parameter, not a router parameter.

BUG=745757

Review-Url: https://codereview.chromium.org/2976393002

milo/buildsource/builder.go

diff --git a/milo/buildsource/builder.go b/milo/buildsource/builder.go
index d085057..40407f5 100644
--- a/milo/buildsource/builder.go
+++ b/milo/buildsource/builder.go
@@ -5,7 +5,6 @@
 package buildsource
 
 import (
-	"strconv"
 	"strings"
 
 	"golang.org/x/net/context"
@@ -13,7 +12,6 @@
 	"github.com/luci/gae/service/datastore"
 
 	"github.com/luci/luci-go/common/errors"
-	"github.com/luci/luci-go/common/logging"
 	"github.com/luci/luci-go/milo/api/resp"
 	"github.com/luci/luci-go/milo/buildsource/buildbot"
 	"github.com/luci/luci-go/milo/buildsource/buildbucket"
@@ -60,22 +58,8 @@
 
 // Get allows you to obtain the resp.Builder that corresponds with this
 // BuilderID.
-func (b BuilderID) Get(c context.Context, limitStr string, cursorStr string) (*resp.Builder, error) {
+func (b BuilderID) Get(c context.Context, limit int, cursorStr string) (*resp.Builder, error) {
 	// TODO(iannucci): replace these implementations with a BuildSummary query.
-
-	limit := 0
-	if limitStr != "" {
-		switch limitVal, err := strconv.ParseInt(limitStr, 10, 0); err {
-		case nil:
-			limit = int(limitVal)
-		default:
-			logging.WithError(err).Warningf(c, "ignoring bad limit %q", limitStr)
-		}
-	}
-	if limit <= 0 {
-		limit = 25
-	}
-
 	source, group, builder, err := b.Split()
 	if err != nil {
 		return nil, err

milo/frontend/middleware.go

diff --git a/milo/frontend/middleware.go b/milo/frontend/middleware.go
index 7c750c2..b728c22 100644
--- a/milo/frontend/middleware.go
+++ b/milo/frontend/middleware.go
@@ -225,26 +225,23 @@
 }
 
 // GetLimit extracts the "limit", "numbuilds", or "num_builds" http param from
-// the request, or returns "-1" implying no limit was specified.
-func GetLimit(r *http.Request) (int, error) {
+// the request, or returns def implying no limit was specified.
+func GetLimit(r *http.Request, def int) int {
 	sLimit := r.FormValue("limit")
 	if sLimit == "" {
 		sLimit = r.FormValue("numbuilds")
 		if sLimit == "" {
 			sLimit = r.FormValue("num_builds")
 			if sLimit == "" {
-				return -1, nil
+				return def
 			}
 		}
 	}
 	limit, err := strconv.Atoi(sLimit)
-	if err != nil {
-		return -1, fmt.Errorf("limit parameter value %q is not a number: %s", sLimit, err)
+	if err != nil || limit < 0 {
+		return def
 	}
-	if limit < 0 {
-		return -1, fmt.Errorf("limit parameter value %q is less than 0", sLimit)
-	}
-	return limit, nil
+	return limit
 }
 
 // pagedURL returns a self URL with the given cursor and limit paging options.
@@ -252,13 +249,7 @@
 // both are unspecified, then limit is omitted.
 func pagedURL(r *http.Request, limit int, cursor string) string {
 	if limit == 0 {
-		var err error
-		limit, err = GetLimit(r)
-		if err != nil {
-			// This should not happen because the handler should've already validated the
-			// limit earlier in the process.
-			panic(err)
-		}
+		limit = GetLimit(r, -1)
 		if limit < 0 {
 			limit = 0
 		}

milo/frontend/view_builder.go

diff --git a/milo/frontend/view_builder.go b/milo/frontend/view_builder.go
index 901bbee..49199a7 100644
--- a/milo/frontend/view_builder.go
+++ b/milo/frontend/view_builder.go
@@ -13,7 +13,11 @@
 // BuilderHandler is responsible for taking a universal builder ID and rendering
 // the builder page (defined in ./appengine/templates/pages/builder.html).
 func BuilderHandler(c *router.Context, builderID buildsource.BuilderID) {
-	builder, err := builderID.Get(c.Context, c.Params.ByName("limit"), c.Params.ByName("cursor"))
+	limit := 25
+	if tLimit := GetLimit(c.Request, -1); tLimit >= 0 {
+		limit = tLimit
+	}
+	builder, err := builderID.Get(c.Context, limit, c.Request.FormValue("cursor"))
 	// TODO(iannucci, hinoka): make MiloBuild refer to annotation stream by
 	// host/prefix/path instead of by directly pulling it. Do all annotation
 	// stream rendering in the frontend.