From: Fuad Muhic <fmuhic@capeannenterprises.com>
Date: Thu, 30 Nov 2017 15:53:12 +0000 (+0100)
Subject: Improved HTTP request filtering and response error handling.
X-Git-Tag: 1.1.3~2^2~88
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/fbb9221d9de4c4a27e0c8aefc48c7c9c7a80ceab

Improved HTTP request filtering and response error handling.
Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>
---

diff --git a/.gitignore b/.gitignore
index 0e876bb6f4..cbebe67bc0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -28,3 +28,4 @@ services/api/config/arvados-clients.yml
 *#*
 .DS_Store
 .vscode
+.Rproj.user
diff --git a/sdk/R/.RData b/sdk/R/.RData
index 167f8c3fa0..13255a2b47 100644
Binary files a/sdk/R/.RData and b/sdk/R/.RData differ
diff --git a/sdk/R/R/Arvados.R b/sdk/R/R/Arvados.R
index b44e105af7..204d87e4e1 100644
--- a/sdk/R/R/Arvados.R
+++ b/sdk/R/R/Arvados.R
@@ -44,6 +44,10 @@ Arvados$methods(
 
         httpParser <- HttpParser()
         collection <- httpParser$parseCollectionGet(server_response)
+
+        if(!is.null(collection$errors))
+            stop(collection$errors)       
+
         class(collection) <- "ArvadosCollection"
 
         return(collection)
@@ -62,11 +66,15 @@ Arvados$methods(
     {
         #Todo(Fudo): Implement limit and offset
         collection_relative_url <- "collections"
-        http_request <- HttpRequest("GET", token, host, collection_relative_url, filters) 
+        http_request <- HttpRequest("GET", token, host, collection_relative_url, filters, limit, offset) 
         server_response <- http_request$execute()
 
         httpParser <- HttpParser()
         collection <- httpParser$parseCollectionGet(server_response)
+
+        if(!is.null(collection$errors))
+            stop(collection$errors)       
+
         class(collection) <- "ArvadosCollectionList"
 
         return(collection)
diff --git a/sdk/R/R/HttpParser.R b/sdk/R/R/HttpParser.R
index c685fb51a8..09304fc599 100644
--- a/sdk/R/R/HttpParser.R
+++ b/sdk/R/R/HttpParser.R
@@ -14,10 +14,6 @@ HttpParser <- setRefClass(
 
         parseCollectionGet = function(server_response) 
         {
-            #Todo(Fudo): Implement proper server code checking
-            #if(server_response$response_code != 200)
-                #stop("Error");
-
             parsed_response <- httr::content(server_response, as = "parsed", type = "application/json")
 
             #Todo(Fudo): Create new Collection object and populate it
diff --git a/sdk/R/R/HttpRequest.R b/sdk/R/R/HttpRequest.R
index 3076eedf9a..96f0a83f21 100644
--- a/sdk/R/R/HttpRequest.R
+++ b/sdk/R/R/HttpRequest.R
@@ -87,28 +87,67 @@ HttpRequest <- setRefClass(
 
         generateQuery = function() 
         {
-            finalQuery <- ""
+            #Todo(Fudo): This function is a mess, refactor it
+            finalQuery <- "?alt=json"
 
             if(!is.null(query_filters))
             {
                 filters <- sapply(query_filters, function(filter)
                 {
-                    filter <- sapply(filter, function(component) 
-                    {
+                    if(length(filter) != 3)
+                        stop("Filter list must have exacthey 3 elements.")
+
+                    attributeAndOperator = filter[c(1, 2)]
+                    filterList = filter[[3]]
+                    filterListIsPrimitive = TRUE
+                    if(length(filterList) > 1)
+                        filterListIsPrimitive = FALSE
+
+                    attributeAndOperator <- sapply(attributeAndOperator, function(component) {
                         component <- paste0("\"", component, "\"")
                     })
-                    
+
+                    filterList <- sapply(unlist(filterList), function(filter) {
+                        filter <- paste0("\"", filter, "\"")
+                    })
+
+                    filterList <- paste(filterList, collapse = ",+")
+
+                    if(!filterListIsPrimitive)
+                        filterList <- paste0("[", filterList, "]")
+
+                    filter <- c(attributeAndOperator, filterList)
+
                     queryParameter <- paste(filter, collapse = ",+")
-                    queryParameter <- paste0("[[", queryParameter, "]]")
+                    queryParameter <- paste0("[", queryParameter, "]")
+        
                 })
 
+                filters <- paste(filters, collapse = ",+")
+                filters <- paste0("[", filters, "]")
+
                 encodedQuery <- URLencode(filters, reserved = T, repeated = T)
 
-                #Todo(Fudo): Hardcoded for now. Look for a better solution.
-                finalQuery <- paste0("?alt=json&filters=", encodedQuery)
+                finalQuery <- paste0(finalQuery, "&filters=", encodedQuery)
 
                 #Todo(Fudo): This is a hack for now. Find a proper solution.
-                finalQuery <- str_replace_all(finalQuery, "%2B", "+")
+                finalQuery <- stringr::str_replace_all(finalQuery, "%2B", "+")
+            }
+
+            if(!is.null(response_limit))
+            {
+                if(!is.numeric(response_limit))
+                    stop("Limit must be a numeric type.")
+                
+                finalQuery <- paste0(finalQuery, "&limit=", response_limit)
+            }
+
+            if(!is.null(query_offset))
+            {
+                if(!is.numeric(query_offset))
+                    stop("Offset must be a numeric type.")
+                
+                finalQuery <- paste0(finalQuery, "&offset=", query_offset)
             }
 
             finalQuery