From 62908a022cf7fc2fe7bf81e9e97f92bd7ef2912c Mon Sep 17 00:00:00 2001
From: Fuad Muhic <muhic.fuad@gmail.com>
Date: Tue, 27 Mar 2018 11:11:18 +0200
Subject: [PATCH] Removed obsolated methods from HttpRequest class and updated
 tests to reflect that change.

Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuhic@capeannenterprises.com>
---
 sdk/R/R/HttpRequest.R                   | 82 +------------------------
 sdk/R/tests/testthat/test-HttpRequest.R | 19 +++---
 2 files changed, 11 insertions(+), 90 deletions(-)

diff --git a/sdk/R/R/HttpRequest.R b/sdk/R/R/HttpRequest.R
index a0e8077bc5..2a5c4fa556 100644
--- a/sdk/R/R/HttpRequest.R
+++ b/sdk/R/R/HttpRequest.R
@@ -15,21 +15,6 @@ HttpRequest <- R6::R6Class(
             self$validVerbs <- c("GET", "POST", "PUT", "DELETE", "PROPFIND", "MOVE")
         },
 
-        execute = function(verb, url, headers = NULL, body = NULL, query = NULL,
-                           limit = NULL, offset = NULL, retryTimes = 0)
-        {
-            if(!(verb %in% self$validVerbs))
-                stop("Http verb is not valid.")
-
-            headers  <- httr::add_headers(unlist(headers))
-            urlQuery <- self$createQuery(query, limit, offset)
-            url      <- paste0(url, urlQuery)
-
-            # times = 1 regular call + numberOfRetries
-            response <- httr::RETRY(verb, url = url, body = body,
-                                    config = headers, times = retryTimes + 1)
-        },
-
         exec = function(verb, url, headers = NULL, body = NULL, queryParams = NULL,
                         retryTimes = 0)
         {
@@ -37,7 +22,7 @@ HttpRequest <- R6::R6Class(
                 stop("Http verb is not valid.")
 
             headers  <- httr::add_headers(unlist(headers))
-            urlQuery <- self$genQuery(queryParams)
+            urlQuery <- self$createQuery(queryParams)
             url      <- paste0(url, urlQuery)
 
             # times = 1 regular call + numberOfRetries
@@ -45,24 +30,7 @@ HttpRequest <- R6::R6Class(
                                     config = headers, times = retryTimes + 1)
         },
 
-        createQuery = function(filters, limit, offset)
-        {
-            finalQuery <- NULL
-
-            finalQuery <- c(finalQuery, private$createFiltersQuery(filters))
-            finalQuery <- c(finalQuery, private$createLimitQuery(limit))
-            finalQuery <- c(finalQuery, private$createOffsetQuery(offset))
-
-            finalQuery <- finalQuery[!is.null(finalQuery)]
-            finalQuery <- paste0(finalQuery, collapse = "&")
-
-            if(finalQuery != "")
-                finalQuery <- paste0("/?", finalQuery)
-
-            finalQuery
-        },
-
-        genQuery = function(queryParams)
+        createQuery = function(queryParams)
         {
             queryParams <- Filter(Negate(is.null), queryParams)
 
@@ -86,51 +54,5 @@ HttpRequest <- R6::R6Class(
         }
     ),
 
-    private = list(
-
-        createFiltersQuery = function(filters)
-        {
-            if(!is.null(filters))
-            {
-                filters <- RListToPythonList(filters, ",")
-                encodedQuery <- URLencode(filters, reserved = T, repeated = T)
-
-                return(paste0("filters=", encodedQuery))
-            }
-
-            return(NULL)
-        },
-
-        createLimitQuery = function(limit)
-        {
-            if(!is.null(limit))
-            {
-                limit <- suppressWarnings(as.numeric(limit))
-
-                if(is.na(limit))
-                    stop("Limit must be a numeric type.")
-                
-                return(paste0("limit=", limit))
-            }
-
-            return(NULL)
-        },
-
-        createOffsetQuery = function(offset)
-        {
-            if(!is.null(offset))
-            {
-                offset <- suppressWarnings(as.numeric(offset))
-
-                if(is.na(offset))
-                    stop("Offset must be a numeric type.")
-                
-                return(paste0("offset=", offset))
-            }
-
-            return(NULL)
-        }
-    ),
-
     cloneable = FALSE
 )
diff --git a/sdk/R/tests/testthat/test-HttpRequest.R b/sdk/R/tests/testthat/test-HttpRequest.R
index 66ab9af196..5c2fb602fd 100644
--- a/sdk/R/tests/testthat/test-HttpRequest.R
+++ b/sdk/R/tests/testthat/test-HttpRequest.R
@@ -4,25 +4,24 @@ context("Http Request")
 test_that("execyte raises exception if http verb is not valid", {
 
     http <- HttpRequest$new()
-    expect_that(http$execute("FAKE VERB", "url"),
+    expect_that(http$exec("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"), {
+test_that("createQuery generates and encodes query portion of http", {
 
     http <- HttpRequest$new()
-    filters <- list(list("color", "=", "red"))
-    limit <- 20
-    offset <- 50
-    expect_that(http$createQuery(filters, limit, offset),
+    queryParams <- list()
+    queryParams$filters <- list(list("color", "=", "red"))
+    queryParams$limit <- 20
+    queryParams$offset <- 50
+    expect_that(http$createQuery(queryParams),
                 equals(paste0("/?filters=%5B%5B%22color%22%2C%22%3D%22%2C%22red",
                               "%22%5D%5D&limit=20&offset=50")))
 }) 
 
-test_that(paste("createQuery generates and empty string",
-                "when filters, limit and offset parameters are set to NULL"), {
+test_that("createQuery generates and empty string when queryParams is an empty list", {
 
     http <- HttpRequest$new()
-    expect_that(http$createQuery(NULL, NULL, NULL), equals(""))
+    expect_that(http$createQuery(list()), equals(""))
 }) 
-- 
2.30.2