13619: Support [uuid, =, ...], cleanups

Remove call to code that looks for cluster_id from json payload of
POST, the SDKs don't generate this (cluster_id goes in the URL query
string or form encoded).

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics.com>

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 04e0e74e3569d07d5d873127ada550d9ae2f552a..72a147ae7e9065a260d341d4af88c349f013925a 100644 (file)
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -155,12 +155,6 @@ func (c *multiClusterQueryResponseCollector) collectResponse(resp *http.Response
 func (h *genericFederatedRequestHandler) handleMultiClusterQuery(w http.ResponseWriter, req *http.Request,
        params url.Values, clusterId *string) bool {
 
-       if !(len(params["count"]) == 1 && (params["count"][0] == `none` ||
-               params["count"][0] == `"none"`)) {
-               // don't federate unless params has count=none
-               return false
-       }
-
        var filters [][]interface{}
        err := json.Unmarshal([]byte(params["filters"][0]), &filters)
        if err != nil {
@@ -173,22 +167,43 @@ func (h *genericFederatedRequestHandler) handleMultiClusterQuery(w http.Response
        if len(filters) == 1 && len(filters[0]) == 3 {
                f1 := filters[0]
                lhs := f1[0].(string)
-               op := f1[1].(string)
-               rhs := f1[2].([]interface{})
-               if lhs == "uuid" && op == "in" {
-                       for _, i := range rhs {
-                               u := i.(string)
-                               *clusterId = u[0:5]
-                               queryClusters[u[0:5]] = append(queryClusters[u[0:5]], u)
+               if lhs == "uuid" {
+                       op, ok := f1[1].(string)
+                       if !ok {
+                               return false
+                       }
+
+                       if op == "in" {
+                               rhs, ok := f1[2].([]interface{})
+                               if ok {
+                                       for _, i := range rhs {
+                                               u := i.(string)
+                                               *clusterId = u[0:5]
+                                               queryClusters[u[0:5]] = append(queryClusters[u[0:5]], u)
+                                       }
+                               }
+                       } else if op == "=" {
+                               u, ok := f1[2].(string)
+                               if ok {
+                                       *clusterId = u[0:5]
+                                       queryClusters[u[0:5]] = append(queryClusters[u[0:5]], u)
+                               }
                        }
                }
+
        }
 
-       if len(queryClusters) == 0 {
-               // Didn't find any ["uuid", "in", ...] filters
+       if len(queryClusters) <= 1 {
+               // Didn't find ["uuid", "in", ...] filters for multiple clusters
                return false
        }
 
+       if !(len(params["count"]) == 1 && (params["count"][0] == `none` ||
+               params["count"][0] == `"none"`)) {
+               httpserver.Error(w, "Federated multi-object query must have count=none", http.StatusBadRequest)
+               return true
+       }
+
        wg := sync.WaitGroup{}
 
        // use channel as a semaphore to limit it to 4
@@ -285,18 +300,10 @@ func (h *genericFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req *h
                clusterId = params["cluster_id"][0]
        }
 
-       // TODO: decide if this actually makes sense...
-       if clusterId == "" && req.Method == "POST" && req.Header.Get("Content-Type") == "application/json" {
-               var hasClusterId struct {
-                       ClusterID string `json:"cluster_id"`
-               }
-               if err = loadParamsFromJson(req, &hasClusterId); err != nil {
-                       httpserver.Error(w, err.Error(), http.StatusBadRequest)
-                       return
-               }
-               clusterId = hasClusterId.ClusterID
-       }
-
+       // Handle the POST-as-GET special case (workaround for large
+       // GET requests that potentially exceed maximum URL length,
+       // like multi-object queries where the filter has 100s of
+       // items)
        effectiveMethod := req.Method
        if req.Method == "POST" && len(params["_method"]) == 1 {
                effectiveMethod = params["_method"][0]

diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index 255adfe92732ffe69763c19ab251d6eb31d1975a..f165aecffc660a913f5fb6b3264ce05bce0092ea 100644 (file)
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -538,31 +538,7 @@ func (s *FederationSuite) TestUpdateRemoteContainerRequest(c *check.C) {
        c.Check(cr.Priority, check.Equals, 696)
 }
 
-func (s *FederationSuite) TestCreateRemoteContainerRequest1(c *check.C) {
-       defer s.localServiceReturns404(c).Close()
-       req := httptest.NewRequest("POST", "/arvados/v1/container_requests",
-               strings.NewReader(`{
-  "cluster_id": "zzzzz",
-  "container_request": {
-    "name": "hello world",
-    "state": "Uncommitted",
-    "output_path": "/",
-    "container_image": "123",
-    "command": ["abc"]
-  }
-}
-`))
-       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
-       req.Header.Set("Content-type", "application/json")
-       resp := s.testRequest(req)
-       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
-       var cr arvados.ContainerRequest
-       c.Check(json.NewDecoder(resp.Body).Decode(&cr), check.IsNil)
-       c.Check(cr.Name, check.Equals, "hello world")
-       c.Check(strings.HasPrefix(cr.UUID, "zzzzz-"), check.Equals, true)
-}
-
-func (s *FederationSuite) TestCreateRemoteContainerRequest2(c *check.C) {
+func (s *FederationSuite) TestCreateRemoteContainerRequest(c *check.C) {
        defer s.localServiceReturns404(c).Close()
        // pass cluster_id via query parameter, this allows arvados-controller
        // to avoid parsing the body