15720: Switch httprouter to gorilla/mux.
authorTom Clegg <tclegg@veritasgenetics.com>
Thu, 14 Nov 2019 19:29:14 +0000 (14:29 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Mon, 18 Nov 2019 16:22:11 +0000 (11:22 -0500)
In httprouter, "/users/:uuid" and "/users/:uuid/activate" are
conflicting routes.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/controller/router/request.go
lib/controller/router/router.go
lib/controller/router/router_test.go
lib/controller/rpc/conn.go
sdk/go/arvados/api.go

index 377f7243c009bef591fffb4b3b53acaf39c0d359..4d18395b6a87b7cc7aa421f78e118c18551453ca 100644 (file)
@@ -13,7 +13,7 @@ import (
        "strconv"
        "strings"
 
-       "github.com/julienschmidt/httprouter"
+       "github.com/gorilla/mux"
 )
 
 // Parse req as an Arvados V1 API request and return the request
@@ -109,9 +109,8 @@ func (rtr *router) loadRequestParams(req *http.Request, attrsKey string) (map[st
                }
        }
 
-       routeParams, _ := req.Context().Value(httprouter.ParamsKey).(httprouter.Params)
-       for _, p := range routeParams {
-               params[p.Key] = p.Value
+       for k, v := range mux.Vars(req) {
+               params[k] = v
        }
 
        if v, ok := params[attrsKey]; ok && attrsKey != "" {
index d3bdce527211e1b26245a820dcbc9cd174f3dc62..709ddfb82eb1c0386dcb17f6748a14b005bbe7bb 100644 (file)
@@ -14,18 +14,18 @@ import (
        "git.curoverse.com/arvados.git/sdk/go/auth"
        "git.curoverse.com/arvados.git/sdk/go/ctxlog"
        "git.curoverse.com/arvados.git/sdk/go/httpserver"
-       "github.com/julienschmidt/httprouter"
+       "github.com/gorilla/mux"
        "github.com/sirupsen/logrus"
 )
 
 type router struct {
-       mux *httprouter.Router
+       mux *mux.Router
        fed arvados.API
 }
 
 func New(fed arvados.API) *router {
        rtr := &router{
-               mux: httprouter.New(),
+               mux: mux.NewRouter(),
                fed: fed,
        }
        rtr.addRoutes()
@@ -214,16 +214,16 @@ func (rtr *router) addRoutes() {
                        rtr.addRoute(endpointPUT, route.defaultOpts, route.exec)
                }
        }
-       rtr.mux.NotFound = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+       rtr.mux.NotFoundHandler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                httpserver.Errors(w, []string{"API endpoint not found"}, http.StatusNotFound)
        })
-       rtr.mux.MethodNotAllowed = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+       rtr.mux.MethodNotAllowedHandler = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                httpserver.Errors(w, []string{"API endpoint not found"}, http.StatusMethodNotAllowed)
        })
 }
 
 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) {
+       rtr.mux.Methods(endpoint.Method).Path("/" + endpoint.Path).HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
                logger := ctxlog.FromContext(req.Context())
                params, err := rtr.loadRequestParams(req, endpoint.AttrsKey)
                if err != nil {
index 3a7045aa4de2681f53af5645d7008d290b035321..6a9fd311ba2b91cd66b03aa4dc7013eb878ea665 100644 (file)
@@ -19,7 +19,7 @@ import (
        "git.curoverse.com/arvados.git/lib/controller/rpc"
        "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
-       "github.com/julienschmidt/httprouter"
+       "github.com/gorilla/mux"
        check "gopkg.in/check.v1"
 )
 
@@ -38,7 +38,7 @@ type RouterSuite struct {
 func (s *RouterSuite) SetUpTest(c *check.C) {
        s.stub = arvadostest.APIStub{}
        s.rtr = &router{
-               mux: httprouter.New(),
+               mux: mux.NewRouter(),
                fed: &s.stub,
        }
        s.rtr.addRoutes()
index 7d7cb486f4f742d57411751254cf7b8dd6ab22ad..afe749fb0d9f3644f1814f7cf763d137bc9af092 100644 (file)
@@ -118,9 +118,9 @@ func (conn *Conn) requestAndDecode(ctx context.Context, dst interface{}, ep arva
                params["reader_tokens"] = tokens[1:]
        }
        path := ep.Path
-       if strings.Contains(ep.Path, "/:uuid") {
+       if strings.Contains(ep.Path, "/{uuid}") {
                uuid, _ := params["uuid"].(string)
-               path = strings.Replace(path, "/:uuid", "/"+uuid, 1)
+               path = strings.Replace(path, "/{uuid}", "/"+uuid, 1)
                delete(params, "uuid")
        }
        return aClient.RequestAndDecodeContext(ctx, dst, ep.Method, path, body, params)
index 5531cf71d344cbb795caaa2cac670d7a3ff88ca1..5de94d73e463ba48a0198df6b4d5c1b912182006 100644 (file)
@@ -20,26 +20,26 @@ var (
        EndpointConfigGet                     = APIEndpoint{"GET", "arvados/v1/config", ""}
        EndpointLogin                         = APIEndpoint{"GET", "login", ""}
        EndpointCollectionCreate              = APIEndpoint{"POST", "arvados/v1/collections", "collection"}
-       EndpointCollectionUpdate              = APIEndpoint{"PATCH", "arvados/v1/collections/:uuid", "collection"}
-       EndpointCollectionGet                 = APIEndpoint{"GET", "arvados/v1/collections/:uuid", ""}
+       EndpointCollectionUpdate              = APIEndpoint{"PATCH", "arvados/v1/collections/{uuid}", "collection"}
+       EndpointCollectionGet                 = APIEndpoint{"GET", "arvados/v1/collections/{uuid}", ""}
        EndpointCollectionList                = APIEndpoint{"GET", "arvados/v1/collections", ""}
-       EndpointCollectionProvenance          = APIEndpoint{"GET", "arvados/v1/collections/:uuid/provenance", ""}
-       EndpointCollectionUsedBy              = APIEndpoint{"GET", "arvados/v1/collections/:uuid/used_by", ""}
-       EndpointCollectionDelete              = APIEndpoint{"DELETE", "arvados/v1/collections/:uuid", ""}
-       EndpointCollectionTrash               = APIEndpoint{"POST", "arvados/v1/collections/:uuid/trash", ""}
-       EndpointCollectionUntrash             = APIEndpoint{"POST", "arvados/v1/collections/:uuid/untrash", ""}
+       EndpointCollectionProvenance          = APIEndpoint{"GET", "arvados/v1/collections/{uuid}/provenance", ""}
+       EndpointCollectionUsedBy              = APIEndpoint{"GET", "arvados/v1/collections/{uuid}/used_by", ""}
+       EndpointCollectionDelete              = APIEndpoint{"DELETE", "arvados/v1/collections/{uuid}", ""}
+       EndpointCollectionTrash               = APIEndpoint{"POST", "arvados/v1/collections/{uuid}/trash", ""}
+       EndpointCollectionUntrash             = APIEndpoint{"POST", "arvados/v1/collections/{uuid}/untrash", ""}
        EndpointSpecimenCreate                = APIEndpoint{"POST", "arvados/v1/specimens", "specimen"}
-       EndpointSpecimenUpdate                = APIEndpoint{"PATCH", "arvados/v1/specimens/:uuid", "specimen"}
-       EndpointSpecimenGet                   = APIEndpoint{"GET", "arvados/v1/specimens/:uuid", ""}
+       EndpointSpecimenUpdate                = APIEndpoint{"PATCH", "arvados/v1/specimens/{uuid}", "specimen"}
+       EndpointSpecimenGet                   = APIEndpoint{"GET", "arvados/v1/specimens/{uuid}", ""}
        EndpointSpecimenList                  = APIEndpoint{"GET", "arvados/v1/specimens", ""}
-       EndpointSpecimenDelete                = APIEndpoint{"DELETE", "arvados/v1/specimens/:uuid", ""}
+       EndpointSpecimenDelete                = APIEndpoint{"DELETE", "arvados/v1/specimens/{uuid}", ""}
        EndpointContainerCreate               = APIEndpoint{"POST", "arvados/v1/containers", "container"}
-       EndpointContainerUpdate               = APIEndpoint{"PATCH", "arvados/v1/containers/:uuid", "container"}
-       EndpointContainerGet                  = APIEndpoint{"GET", "arvados/v1/containers/:uuid", ""}
+       EndpointContainerUpdate               = APIEndpoint{"PATCH", "arvados/v1/containers/{uuid}", "container"}
+       EndpointContainerGet                  = APIEndpoint{"GET", "arvados/v1/containers/{uuid}", ""}
        EndpointContainerList                 = APIEndpoint{"GET", "arvados/v1/containers", ""}
-       EndpointContainerDelete               = APIEndpoint{"DELETE", "arvados/v1/containers/:uuid", ""}
-       EndpointContainerLock                 = APIEndpoint{"POST", "arvados/v1/containers/:uuid/lock", ""}
-       EndpointContainerUnlock               = APIEndpoint{"POST", "arvados/v1/containers/:uuid/unlock", ""}
+       EndpointContainerDelete               = APIEndpoint{"DELETE", "arvados/v1/containers/{uuid}", ""}
+       EndpointContainerLock                 = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/lock", ""}
+       EndpointContainerUnlock               = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/unlock", ""}
        EndpointAPIClientAuthorizationCurrent = APIEndpoint{"GET", "arvados/v1/api_client_authorizations/current", ""}
 )