Reworkder HttpRequest class to use RETRY functionality

Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>

diff --git a/sdk/R/R/HttpRequest.R b/sdk/R/R/HttpRequest.R
index ec364e5420ae15903ae478f17c36f2ca2d39a58b..53c8e224a8ff97863ef92fe1baeab6a190a045aa 100644 (file)
--- a/sdk/R/R/HttpRequest.R
+++ b/sdk/R/R/HttpRequest.R
@@ -7,67 +7,26 @@ HttpRequest <- R6::R6Class(
     public = list(
 
         validContentTypes = NULL,
+        validVerbs = NULL,
 
         initialize = function() 
         {
             self$validContentTypes <- c("text", "raw")
+            self$validVerbs <- c("GET", "POST", "PUT", "DELETE", "PROPFIND", "MOVE")
         },
 
-        GET = function(url, headers = NULL, queryFilters = NULL, limit = NULL, offset = NULL)
+        execute = function(verb, url, headers = NULL, body = NULL, query = NULL,
+                           limit = NULL, offset = NULL, retryTimes = 3)
         {
-            headers <- httr::add_headers(unlist(headers))
-            query <- self$createQuery(queryFilters, limit, offset)
-            url <- paste0(url, query)
+            if(!(verb %in% self$validVerbs))
+                stop("Http verb is not valid.")
 
-            serverResponse <- httr::GET(url = url, config = headers)
-        },
-
-        PUT = function(url, headers = NULL, body = NULL,
-                       queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            headers <- httr::add_headers(unlist(headers))
-            query <- self$createQuery(queryFilters, limit, offset)
-            url <- paste0(url, query)
-
-            serverResponse <- httr::PUT(url = url, config = headers, body = body)
-        },
-
-        POST = function(url, headers = NULL, body = NULL,
-                        queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            headers <- httr::add_headers(unlist(headers))
-            query <- self$createQuery(queryFilters, limit, offset)
-            url <- paste0(url, query)
-
-            serverResponse <- httr::POST(url = url, config = headers, body = body)
-        },
-
-        DELETE = function(url, headers = NULL, body = NULL,
-                          queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            headers <- httr::add_headers(unlist(headers))
-            query <- self$createQuery(queryFilters, limit, offset)
-            url <- paste0(url, query)
-
-            serverResponse <- httr::DELETE(url = url, config = headers)
-        },
-
-        PROPFIND = function(url, headers = NULL)
-        {
-            h <- curl::new_handle()
-            curl::handle_setopt(h, customrequest = "PROPFIND")
-            curl::handle_setheaders(h, .list = headers)
-
-            propfindResponse <- curl::curl_fetch_memory(url, h)
-        },
-
-        MOVE = function(url, headers = NULL)
-        {
-            h <- curl::new_handle()
-            curl::handle_setopt(h, customrequest = "MOVE")
-            curl::handle_setheaders(h, .list = headers)
+            headers  <- httr::add_headers(unlist(headers))
+            urlQuery <- self$createQuery(query, limit, offset)
+            url      <- paste0(url, urlQuery)
 
-            propfindResponse <- curl::curl_fetch_memory(url, h)
+            response <- httr::RETRY(verb, url = url, body = body,
+                                    config = headers, times = retryTimes)
         },
 
         createQuery = function(filters, limit, offset)

diff --git a/sdk/R/R/RESTService.R b/sdk/R/R/RESTService.R
index 62356b76384094a8ba0d222dedd9ec77d2fb7f03..74031b00c35ed4fcd3eb7e1ad532922c4397dba8 100644 (file)
--- a/sdk/R/R/RESTService.R
+++ b/sdk/R/R/RESTService.R
@@ -4,10 +4,10 @@ RESTService <- R6::R6Class(
 
     public = list(
 
-        hostName       = NULL,
-        token          = NULL,
-        http           = NULL,
-        httpParser     = NULL,
+        hostName   = NULL,
+        token      = NULL,
+        http       = NULL,
+        httpParser = NULL,
 
         initialize = function(token, hostName, webDavHostName = NULL,  http, httpParser)
         {
@@ -32,7 +32,7 @@ RESTService <- R6::R6Class(
 
                 headers <- list(Authorization = paste("OAuth2", self$token))
 
-                serverResponse <- self$http$GET(discoveryDocumentURL, headers)
+                serverResponse <- self$http$execute("GET", discoveryDocumentURL, headers)
 
                 discoveryDocument <- self$httpParser$parseJSONResponse(serverResponse)
                 private$webDavHostName <- discoveryDocument$keepWebServiceUrl
@@ -49,7 +49,7 @@ RESTService <- R6::R6Class(
             resourceURL <- paste0(self$hostName, resource, "/", uuid)
             headers <- list(Authorization = paste("OAuth2", self$token))
 
-            serverResponse <- self$http$GET(resourceURL, headers)
+            serverResponse <- self$http$execute("GET", resourceURL, headers)
 
             resource <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -63,9 +63,10 @@ RESTService <- R6::R6Class(
         {
             resourceURL <- paste0(self$hostName, resource)
             headers <- list(Authorization = paste("OAuth2", self$token))
+            body <- NULL
 
-            serverResponse <- self$http$GET(resourceURL, headers, filters,
-                                            limit, offset)
+            serverResponse <- self$http$execute("GET", resourceURL, headers, body,
+                                                filters, limit, offset)
 
             resources <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -84,11 +85,13 @@ RESTService <- R6::R6Class(
             items <- c()
             while(length(items) < itemsAvailable)
             {
-                serverResponse <- self$http$GET(url          = resourceURL,
-                                                headers      = headers,
-                                                queryFilters = filters,
-                                                limit        = NULL,
-                                                offset       = offset)
+                serverResponse <- self$http$execute(verb    = "GET",
+                                                    url     = resourceURL,
+                                                    headers = headers,
+                                                    body    = NULL,
+                                                    query   = filters,
+                                                    limit   = NULL,
+                                                    offset  = offset)
 
                 parsedResponse <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -109,7 +112,7 @@ RESTService <- R6::R6Class(
             headers <- list("Authorization" = paste("OAuth2", self$token),
                             "Content-Type"  = "application/json")
 
-            serverResponse <- self$http$DELETE(collectionURL, headers)
+            serverResponse <- self$http$execute("DELETE", collectionURL, headers)
 
             removedResource <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -127,7 +130,7 @@ RESTService <- R6::R6Class(
 
             newContent <- jsonlite::toJSON(newContent, auto_unbox = T)
 
-            serverResponse <- self$http$PUT(resourceURL, headers, newContent)
+            serverResponse <- self$http$execute("PUT", resourceURL, headers, newContent)
 
             updatedResource <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -145,7 +148,7 @@ RESTService <- R6::R6Class(
 
             content <- jsonlite::toJSON(content, auto_unbox = T)
 
-            serverResponse <- self$http$POST(resourceURL, headers, content)
+            serverResponse <- self$http$execute("POST", resourceURL, headers, content)
 
             newResource <- self$httpParser$parseJSONResponse(serverResponse)
 
@@ -169,7 +172,7 @@ RESTService <- R6::R6Class(
                               uuid, "/", relativePath);
             headers <- list(Authorization = paste("OAuth2", self$token)) 
 
-            serverResponse <- self$http$DELETE(fileURL, headers)
+            serverResponse <- self$http$execute("DELETE", fileURL, headers)
 
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
@@ -186,7 +189,7 @@ RESTService <- R6::R6Class(
             headers <- list("Authorization" = paste("OAuth2", self$token),
                            "Destination" = toURL)
 
-            serverResponse <- self$http$MOVE(fromURL, headers)
+            serverResponse <- self$http$execute("MOVE", fromURL, headers)
 
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
@@ -201,7 +204,7 @@ RESTService <- R6::R6Class(
 
             headers <- list("Authorization" = paste("OAuth2", self$token))
 
-            response <- self$http$PROPFIND(collectionURL, headers)
+            response <- self$http$execute("PROPFIND", collectionURL, headers)
 
             if(all(response == ""))
                 stop("Response is empty, request may be misconfigured")
@@ -221,7 +224,7 @@ RESTService <- R6::R6Class(
 
             headers <- list("Authorization" = paste("OAuth2", self$token))
 
-            response <- self$http$PROPFIND(subcollectionURL, headers)
+            response <- self$http$execute("PROPFIND", subcollectionURL, headers)
 
             if(all(response == ""))
                 stop("Response is empty, request may be misconfigured")
@@ -257,7 +260,7 @@ RESTService <- R6::R6Class(
             if(!(contentType %in% self$httpParser$validContentTypes))
                 stop("Invalid contentType. Please use text or raw.")
 
-            serverResponse <- self$http$GET(fileURL, headers)
+            serverResponse <- self$http$execute("GET", fileURL, headers)
 
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
@@ -273,7 +276,7 @@ RESTService <- R6::R6Class(
                             "Content-Type" = contentType)
             body <- content
 
-            serverResponse <- self$http$PUT(fileURL, headers, body)
+            serverResponse <- self$http$execute("PUT", fileURL, headers, body)
 
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))
@@ -309,7 +312,7 @@ RESTService <- R6::R6Class(
                             "Content-Type" = contentType)
             body <- NULL
 
-            serverResponse <- self$http$PUT(fileURL, headers, body)
+            serverResponse <- self$http$execute("PUT", fileURL, headers, body)
 
             if(serverResponse$status_code < 200 || serverResponse$status_code >= 300)
                 stop(paste("Server code:", serverResponse$status_code))

diff --git a/sdk/R/tests/testthat/fakes/FakeHttpRequest.R b/sdk/R/tests/testthat/fakes/FakeHttpRequest.R
index 34dde62e786526d322606adbaffdcff69d09a604..c46e03bae933ed10fb5492eec946841fc6625071 100644 (file)
--- a/sdk/R/tests/testthat/fakes/FakeHttpRequest.R
+++ b/sdk/R/tests/testthat/fakes/FakeHttpRequest.R
@@ -36,85 +36,53 @@ FakeHttpRequest <- R6::R6Class(
             else
                 self$content <- serverResponse
 
-            self$expectedURL <- expectedURL
-            self$URLIsProperlyConfigured <- FALSE
-            self$expectedQueryFilters <- expectedFilters
-            self$queryFiltersAreCorrect <- FALSE
+            self$expectedURL                             <- expectedURL
+            self$URLIsProperlyConfigured                 <- FALSE
+            self$expectedQueryFilters                    <- expectedFilters
+            self$queryFiltersAreCorrect                  <- FALSE
             self$requestHeaderContainsAuthorizationField <- FALSE
-            self$requestHeaderContainsDestinationField <- FALSE
-            self$requestHeaderContainsRangeField <- FALSE
-            self$requestHeaderContainsContentTypeField <- FALSE
-            self$JSONEncodedBodyIsProvided <- FALSE
-            self$requestBodyIsProvided <- FALSE
+            self$requestHeaderContainsDestinationField   <- FALSE
+            self$requestHeaderContainsRangeField         <- FALSE
+            self$requestHeaderContainsContentTypeField   <- FALSE
+            self$JSONEncodedBodyIsProvided               <- FALSE
+            self$requestBodyIsProvided                   <- FALSE
 
-            self$numberOfGETRequests <- 0
+            self$numberOfGETRequests    <- 0
             self$numberOfDELETERequests <- 0
-            self$numberOfPUTRequests <- 0
-            self$numberOfPOSTRequests <- 0
-            self$numberOfMOVERequests <- 0
+            self$numberOfPUTRequests    <- 0
+            self$numberOfPOSTRequests   <- 0
+            self$numberOfMOVERequests   <- 0
 
             self$serverMaxElementsPerRequest <- 5
         },
 
-        GET = function(url, headers = NULL, queryFilters = NULL, limit = NULL, offset = NULL)
+        execute = function(verb, url, headers = NULL, body = NULL, query = NULL,
+                           limit = NULL, offset = NULL, retryTimes = 3)
         {
             private$validateURL(url)
             private$validateHeaders(headers)
             private$validateFilters(queryFilters)
-            self$numberOfGETRequests <- self$numberOfGETRequests + 1
+            private$validateBody(body)
 
-            if(!is.null(self$content$items_available))
+            if(verb == "GET")
+                self$numberOfGETRequests <- self$numberOfGETRequests + 1
+            else if(verb == "POST")
+                self$numberOfPOSTRequests <- self$numberOfPOSTRequests + 1
+            else if(verb == "PUT")
+                self$numberOfPUTRequests <- self$numberOfPUTRequests + 1
+            else if(verb == "DELETE")
+                self$numberOfDELETERequests <- self$numberOfDELETERequests + 1
+            else if(verb == "MOVE")
+                self$numberOfMOVERequests <- self$numberOfMOVERequests + 1
+            else if(verb == "PROPFIND")
             {
-                return(private$getElements(offset, limit))
+                return(self$content)
             }
+
+            if(!is.null(self$content$items_available))
+                return(private$getElements(offset, limit))
             else
                 return(self$content)
-        },
-
-        PUT = function(url, headers = NULL, body = NULL,
-                       queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            private$validateURL(url)
-            private$validateHeaders(headers)
-            private$validateBody(body)
-            self$numberOfPUTRequests <- self$numberOfPUTRequests + 1
-
-            self$content
-        },
-
-        POST = function(url, headers = NULL, body = NULL,
-                        queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            private$validateURL(url)
-            private$validateHeaders(headers)
-            private$validateBody(body)
-            self$numberOfPOSTRequests <- self$numberOfPOSTRequests + 1
-
-            self$content
-        },
-
-        DELETE = function(url, headers = NULL, body = NULL,
-                          queryFilters = NULL, limit = NULL, offset = NULL)
-        {
-            private$validateURL(url)
-            private$validateHeaders(headers)
-            self$numberOfDELETERequests <- self$numberOfDELETERequests + 1
-            self$content
-        },
-
-        PROPFIND = function(url, headers = NULL)
-        {
-            private$validateURL(url)
-            private$validateHeaders(headers)
-            self$content
-        },
-
-        MOVE = function(url, headers = NULL)
-        {
-            private$validateURL(url)
-            private$validateHeaders(headers)
-            self$numberOfMOVERequests <- self$numberOfMOVERequests + 1
-            self$content
         }
     ),
 
@@ -143,9 +111,6 @@ FakeHttpRequest <- R6::R6Class(
 
         validateBody = function(body)
         {
-            if(!is.null(body) && class(body) == "json")           
-                self$JSONEncodedBodyIsProvided <- TRUE
-
             if(!is.null(body))           
             {
                 self$requestBodyIsProvided <- TRUE

diff --git a/sdk/R/tests/testthat/test-HttpRequest.R b/sdk/R/tests/testthat/test-HttpRequest.R
index e85037465b50012051827b83a9d3f4f45f929c60..66ab9af19636a33f96e257eebbdf3fa0813add72 100644 (file)
--- a/sdk/R/tests/testthat/test-HttpRequest.R
+++ b/sdk/R/tests/testthat/test-HttpRequest.R
@@ -1,6 +1,13 @@
 context("Http Request")
 
 
+test_that("execyte raises exception if http verb is not valid", {
+
+    http <- HttpRequest$new()
+    expect_that(http$execute("FAKE VERB", "url"),
+               throws_error("Http verb is not valid."))
+}) 
+
 test_that(paste("createQuery generates and encodes query portion of http",
                 "request based on filters, limit and offset parameters"), {