[ARVADOS] updated: 1.3.0-1159-g03977060b

Git user git at public.curoverse.com
Mon Jun 24 15:22:34 UTC 2019


Summary of changes:
 .../router/{checker.go => checker_test.go}         |   5 +
 lib/controller/router/request.go                   |  33 +++++-
 lib/controller/router/response.go                  |   1 +
 lib/controller/router/router.go                    | 113 +++++++++++----------
 4 files changed, 95 insertions(+), 57 deletions(-)
 rename lib/controller/router/{checker.go => checker_test.go} (73%)

       via  03977060ba2024b4f3bbc0146726b609f1915caf (commit)
       via  2a4c07b8a031b64714cd02b5e3caec413c849a31 (commit)
       via  8db3f9518141f04a29f4edc7a232c8d9fe4d7c1a (commit)
       via  900341fef93f801eae8ac60d9e1c94477b2f6ea1 (commit)
      from  9f6a124c856292511726e679ebfbb96e0d640046 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 03977060ba2024b4f3bbc0146726b609f1915caf
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jun 24 11:21:30 2019 -0400

    14287: Extract inline func.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 909667c54..f846f2dcd 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -33,11 +33,13 @@ func New(cluster *arvados.Cluster) *router {
 	return rtr
 }
 
+type routableFunc func(ctx context.Context, opts interface{}) (interface{}, error)
+
 func (rtr *router) addRoutes() {
 	for _, route := range []struct {
 		endpoint    arvados.APIEndpoint
 		defaultOpts func() interface{}
-		exec        func(ctx context.Context, opts interface{}) (interface{}, error)
+		exec        routableFunc
 	}{
 		{
 			arvados.EndpointCollectionCreate,
@@ -191,58 +193,12 @@ func (rtr *router) addRoutes() {
 			},
 		},
 	} {
-		route := route
-		methods := []string{route.endpoint.Method}
+		rtr.addRoute(route.endpoint, route.defaultOpts, route.exec)
 		if route.endpoint.Method == "PATCH" {
-			methods = append(methods, "PUT")
-		}
-		for _, method := range methods {
-			rtr.mux.HandlerFunc(method, "/"+route.endpoint.Path, func(w http.ResponseWriter, req *http.Request) {
-				logger := ctxlog.FromContext(req.Context())
-				params, err := rtr.loadRequestParams(req, route.endpoint.AttrsKey)
-				if err != nil {
-					logger.WithField("req", req).WithField("route", route).WithError(err).Debug("error loading request params")
-					rtr.sendError(w, err)
-					return
-				}
-				opts := route.defaultOpts()
-				err = rtr.transcode(params, opts)
-				if err != nil {
-					logger.WithField("params", params).WithError(err).Debugf("error transcoding params to %T", opts)
-					rtr.sendError(w, err)
-					return
-				}
-				respOpts, err := rtr.responseOptions(opts)
-				if err != nil {
-					logger.WithField("opts", opts).WithError(err).Debugf("error getting response options from %T", opts)
-					rtr.sendError(w, err)
-					return
-				}
-
-				creds := auth.CredentialsFromRequest(req)
-				if rt, _ := params["reader_tokens"].([]interface{}); len(rt) > 0 {
-					for _, t := range rt {
-						if t, ok := t.(string); ok {
-							creds.Tokens = append(creds.Tokens, t)
-						}
-					}
-				}
-				ctx := req.Context()
-				ctx = context.WithValue(ctx, auth.ContextKeyCredentials, creds)
-				ctx = arvados.ContextWithRequestID(ctx, req.Header.Get("X-Request-Id"))
-				logger.WithFields(logrus.Fields{
-					"apiEndpoint": route.endpoint,
-					"apiOptsType": fmt.Sprintf("%T", opts),
-					"apiOpts":     opts,
-				}).Debug("exec")
-				resp, err := route.exec(ctx, opts)
-				if err != nil {
-					logger.WithError(err).Debugf("returning error type %T", err)
-					rtr.sendError(w, err)
-					return
-				}
-				rtr.sendResponse(w, resp, respOpts)
-			})
+			// Accept PUT as a synonym for PATCH.
+			endpointPUT := route.endpoint
+			endpointPUT.Method = "PUT"
+			rtr.addRoute(endpointPUT, route.defaultOpts, route.exec)
 		}
 	}
 	rtr.mux.NotFound = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
@@ -253,6 +209,59 @@ func (rtr *router) addRoutes() {
 	})
 }
 
+func (rtr *router) addRoute(endpoint arvados.APIEndpoint, defaultOpts func() interface{}, exec routableFunc) {
+	rtr.mux.HandlerFunc(endpoint.Method, "/"+endpoint.Path, func(w http.ResponseWriter, req *http.Request) {
+		logger := ctxlog.FromContext(req.Context())
+		params, err := rtr.loadRequestParams(req, endpoint.AttrsKey)
+		if err != nil {
+			logger.WithFields(logrus.Fields{
+				"req":      req,
+				"method":   endpoint.Method,
+				"endpoint": endpoint,
+			}).WithError(err).Debug("error loading request params")
+			rtr.sendError(w, err)
+			return
+		}
+		opts := defaultOpts()
+		err = rtr.transcode(params, opts)
+		if err != nil {
+			logger.WithField("params", params).WithError(err).Debugf("error transcoding params to %T", opts)
+			rtr.sendError(w, err)
+			return
+		}
+		respOpts, err := rtr.responseOptions(opts)
+		if err != nil {
+			logger.WithField("opts", opts).WithError(err).Debugf("error getting response options from %T", opts)
+			rtr.sendError(w, err)
+			return
+		}
+
+		creds := auth.CredentialsFromRequest(req)
+		if rt, _ := params["reader_tokens"].([]interface{}); len(rt) > 0 {
+			for _, t := range rt {
+				if t, ok := t.(string); ok {
+					creds.Tokens = append(creds.Tokens, t)
+				}
+			}
+		}
+		ctx := req.Context()
+		ctx = context.WithValue(ctx, auth.ContextKeyCredentials, creds)
+		ctx = arvados.ContextWithRequestID(ctx, req.Header.Get("X-Request-Id"))
+		logger.WithFields(logrus.Fields{
+			"apiEndpoint": endpoint,
+			"apiOptsType": fmt.Sprintf("%T", opts),
+			"apiOpts":     opts,
+		}).Debug("exec")
+		resp, err := exec(ctx, opts)
+		if err != nil {
+			logger.WithError(err).Debugf("returning error type %T", err)
+			rtr.sendError(w, err)
+			return
+		}
+		rtr.sendResponse(w, resp, respOpts)
+	})
+}
+
 func (rtr *router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	switch strings.SplitN(strings.TrimLeft(r.URL.Path, "/"), "/", 2)[0] {
 	case "login", "logout", "auth":

commit 2a4c07b8a031b64714cd02b5e3caec413c849a31
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jun 24 11:20:55 2019 -0400

    14287: Specify response Content-Type.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/controller/router/response.go b/lib/controller/router/response.go
index 995fb01ff..aa3af1f64 100644
--- a/lib/controller/router/response.go
+++ b/lib/controller/router/response.go
@@ -120,6 +120,7 @@ func (rtr *router) sendResponse(w http.ResponseWriter, resp interface{}, opts re
 			tmp[k] = t.Format(rfc3339NanoFixed)
 		}
 	}
+	w.Header().Set("Content-Type", "application/json")
 	json.NewEncoder(w).Encode(tmp)
 }
 

commit 8db3f9518141f04a29f4edc7a232c8d9fe4d7c1a
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jun 24 11:20:25 2019 -0400

    14287: Add comments to request param parsing.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/controller/router/request.go b/lib/controller/router/request.go
index 47d8bb110..c219b1e1d 100644
--- a/lib/controller/router/request.go
+++ b/lib/controller/router/request.go
@@ -27,9 +27,32 @@ func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[st
 		return nil, httpError(http.StatusBadRequest, err)
 	}
 	params := map[string]interface{}{}
+
+	// Load parameters from req.Form, which (after
+	// req.ParseForm()) includes the query string and -- when
+	// Content-Type is application/x-www-form-urlencoded -- the
+	// request body.
 	for k, values := range req.Form {
+		// All of these form values arrive as strings, so we
+		// need some type-guessing to accept non-string
+		// inputs:
+		//
+		// Values for parameters that take ints (limit=1) or
+		// bools (include_trash=1) are parsed accordingly.
+		//
+		// "null" and "" are nil.
+		//
+		// Values that look like JSON objects, arrays, or
+		// strings are parsed as JSON.
+		//
+		// The rest are left as strings.
 		for _, v := range values {
 			switch {
+			case intParams[k]:
+				params[k], err = strconv.ParseInt(v, 10, 64)
+				if err != nil {
+					return nil, err
+				}
 			case boolParams[k]:
 				params[k] = stringToBool(v)
 			case v == "null" || v == "":
@@ -55,11 +78,6 @@ func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[st
 					return nil, err
 				}
 				params[k] = j
-			case k == "limit" || k == "offset":
-				params[k], err = strconv.ParseInt(v, 10, 64)
-				if err != nil {
-					return nil, err
-				}
 			default:
 				params[k] = v
 			}
@@ -140,6 +158,11 @@ func (rtr *router) transcode(src interface{}, dst interface{}) error {
 	return err
 }
 
+var intParams = map[string]bool{
+	"limit":  true,
+	"offset": true,
+}
+
 var boolParams = map[string]bool{
 	"distinct":             true,
 	"ensure_unique_name":   true,

commit 900341fef93f801eae8ac60d9e1c94477b2f6ea1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Jun 24 09:21:08 2019 -0400

    14287: Comment and rename checker.go.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/controller/router/checker.go b/lib/controller/router/checker_test.go
similarity index 73%
rename from lib/controller/router/checker.go
rename to lib/controller/router/checker_test.go
index b9fec9168..93d51fa9d 100644
--- a/lib/controller/router/checker.go
+++ b/lib/controller/router/checker_test.go
@@ -11,6 +11,11 @@ import (
 	check "gopkg.in/check.v1"
 )
 
+// a Gocheck checker for testing the name of a function. Used with
+// (*arvadostest.APIStub).Calls() to check that an HTTP request has
+// been routed to the correct arvados.API method.
+//
+//	c.Check(bytes.NewBuffer().Read, isMethodNamed, "Read")
 var isMethodNamed check.Checker = &chkIsMethodNamed{
 	CheckerInfo: &check.CheckerInfo{
 		Name:   "isMethodNamed",

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list