From 69274f95798389376e00de7f07a9822858c3ee62 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Wed, 23 Sep 2020 16:17:09 -0400 Subject: [PATCH] 16827: Fix getFileNamesFromResponse Update tests. Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- sdk/R/R/HttpParser.R | 5 +- sdk/R/R/RESTService.R | 15 +++--- sdk/R/R/autoGenAPI.R | 2 +- sdk/R/tests/testthat/test-ArvadosFile.R | 12 ++--- sdk/R/tests/testthat/test-Collection.R | 22 ++++----- sdk/R/tests/testthat/test-CollectionTree.R | 22 ++++----- sdk/R/tests/testthat/test-HttpParser.R | 8 ++-- sdk/R/tests/testthat/test-HttpRequest.R | 12 ++--- sdk/R/tests/testthat/test-RESTService.R | 54 +++++++++++----------- sdk/R/tests/testthat/test-Subcollection.R | 28 +++++------ 10 files changed, 88 insertions(+), 92 deletions(-) diff --git a/sdk/R/R/HttpParser.R b/sdk/R/R/HttpParser.R index cd492166a1..60bf782827 100644 --- a/sdk/R/R/HttpParser.R +++ b/sdk/R/R/HttpParser.R @@ -31,14 +31,13 @@ HttpParser <- R6::R6Class( { text <- rawToChar(response$content) doc <- XML::xmlParse(text, asText=TRUE) - base <- paste(paste("/", strsplit(uri, "/")[[1]][-1:-3], sep="", collapse=""), "/", sep="") + base <- paste("/", strsplit(uri, "/")[[1]][4], "/", sep="") result <- unlist( XML::xpathApply(doc, "//D:response/D:href", function(node) { sub(base, "", URLdecode(XML::xmlValue(node)), fixed=TRUE) }) ) - result <- result[result != ""] - result[-1] + result[result != ""] }, getFileSizesFromResponse = function(response, uri) diff --git a/sdk/R/R/RESTService.R b/sdk/R/R/RESTService.R index 78b2c35e32..9c65e72861 100644 --- a/sdk/R/R/RESTService.R +++ b/sdk/R/R/RESTService.R @@ -36,16 +36,13 @@ RESTService <- R6::R6Class( { if(is.null(private$webDavHostName)) { - discoveryDocumentURL <- paste0("https://", private$rawHostName, - "/discovery/v1/apis/arvados/v1/rest") + publicConfigURL <- paste0("https://", private$rawHostName, + "/arvados/v1/config") - headers <- list(Authorization = paste("OAuth2", self$token)) - - serverResponse <- self$http$exec("GET", discoveryDocumentURL, headers, - retryTimes = self$numRetries) + serverResponse <- self$http$exec("GET", publicConfigURL, retryTimes = self$numRetries) - discoveryDocument <- self$httpParser$parseJSONResponse(serverResponse) - private$webDavHostName <- discoveryDocument$keepWebServiceUrl + configDocument <- self$httpParser$parseJSONResponse(serverResponse) + private$webDavHostName <- configDocument$Services$WebDAVDownload$ExternalURL if(is.null(private$webDavHostName)) stop("Unable to find WebDAV server.") @@ -118,7 +115,7 @@ RESTService <- R6::R6Class( collectionURL <- URLencode(paste0(self$getWebDavHostName(), "c=", uuid)) - headers <- list("Authorization" = paste("OAuth2", self$token)) + headers <- list("Authorization" = paste("Bearer", self$token)) response <- self$http$exec("PROPFIND", collectionURL, headers, retryTimes = self$numRetries) diff --git a/sdk/R/R/autoGenAPI.R b/sdk/R/R/autoGenAPI.R index 1aef20b6cb..aea64323b5 100644 --- a/sdk/R/R/autoGenAPI.R +++ b/sdk/R/R/autoGenAPI.R @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 getAPIDocument <- function(){ - url <- "https://4xphq.arvadosapi.com/discovery/v1/apis/arvados/v1/rest" + url <- "https://ce8i5.arvadosapi.com/discovery/v1/apis/arvados/v1/rest" serverResponse <- httr::RETRY("GET", url = url) httr::content(serverResponse, as = "parsed", type = "application/json") diff --git a/sdk/R/tests/testthat/test-ArvadosFile.R b/sdk/R/tests/testthat/test-ArvadosFile.R index e3457c993f..da7d52c67d 100644 --- a/sdk/R/tests/testthat/test-ArvadosFile.R +++ b/sdk/R/tests/testthat/test-ArvadosFile.R @@ -23,7 +23,7 @@ test_that("get always returns NULL", { dog <- ArvadosFile$new("dog") responseIsNull <- is.null(dog$get("something")) - expect_that(responseIsNull, is_true()) + expect_true(responseIsNull) }) test_that("getFirst always returns NULL", { @@ -31,7 +31,7 @@ test_that("getFirst always returns NULL", { dog <- ArvadosFile$new("dog") responseIsNull <- is.null(dog$getFirst()) - expect_that(responseIsNull, is_true()) + expect_true(responseIsNull) }) test_that(paste("getSizeInBytes returns zero if arvadosFile", @@ -266,8 +266,8 @@ test_that("move moves arvados file inside collection tree", { dogIsNullOnOldLocation <- is.null(collection$get("animal/dog")) dogExistsOnNewLocation <- !is.null(collection$get("dog")) - expect_that(dogIsNullOnOldLocation, is_true()) - expect_that(dogExistsOnNewLocation, is_true()) + expect_true(dogIsNullOnOldLocation) + expect_true(dogExistsOnNewLocation) }) test_that(paste("copy raises exception if arvados file", @@ -339,8 +339,8 @@ test_that("copy copies arvados file inside collection tree", { dogExistsOnOldLocation <- !is.null(collection$get("animal/dog")) dogExistsOnNewLocation <- !is.null(collection$get("dog")) - expect_that(dogExistsOnOldLocation, is_true()) - expect_that(dogExistsOnNewLocation, is_true()) + expect_true(dogExistsOnOldLocation) + expect_true(dogExistsOnNewLocation) }) test_that("duplicate performs deep cloning of Arvados file", { diff --git a/sdk/R/tests/testthat/test-Collection.R b/sdk/R/tests/testthat/test-Collection.R index 636359ae21..20a2ecf05b 100644 --- a/sdk/R/tests/testthat/test-Collection.R +++ b/sdk/R/tests/testthat/test-Collection.R @@ -86,7 +86,7 @@ test_that(paste("add adds ArvadosFile or Subcollection", dog <- collection$get("animal/dog") dogExistsInCollection <- !is.null(dog) && dog$getName() == "dog" - expect_that(dogExistsInCollection, is_true()) + expect_true(dogExistsInCollection) expect_that(fakeREST$createCallCount, equals(1)) }) @@ -119,8 +119,8 @@ test_that(paste("create adds files specified by fileNames", dogExistsInCollection <- !is.null(dog) && dog$getName() == "dog" catExistsInCollection <- !is.null(cat) && cat$getName() == "cat" - expect_that(dogExistsInCollection, is_true()) - expect_that(catExistsInCollection, is_true()) + expect_true(dogExistsInCollection) + expect_true(catExistsInCollection) expect_that(fakeREST$createCallCount, equals(2)) }) @@ -168,8 +168,8 @@ test_that(paste("remove removes files specified by paths", dogExistsInCollection <- !is.null(dog) && dog$getName() == "dog" catExistsInCollection <- !is.null(cat) && cat$getName() == "cat" - expect_that(dogExistsInCollection, is_false()) - expect_that(catExistsInCollection, is_false()) + expect_false(dogExistsInCollection) + expect_false(catExistsInCollection) expect_that(fakeREST$deleteCallCount, equals(2)) }) @@ -188,8 +188,8 @@ test_that(paste("move moves content to a new location inside file tree", dogIsNullOnOldLocation <- is.null(collection$get("animal/dog")) dogExistsOnNewLocation <- !is.null(collection$get("dog")) - expect_that(dogIsNullOnOldLocation, is_true()) - expect_that(dogExistsOnNewLocation, is_true()) + expect_true(dogIsNullOnOldLocation) + expect_true(dogExistsOnNewLocation) expect_that(fakeREST$moveCallCount, equals(1)) }) @@ -219,7 +219,7 @@ test_that("getFileListing returns sorted collection content received from REST s contentMatchExpected <- all(collection$getFileListing() == c("animal", "animal/fish", "ball")) - expect_that(contentMatchExpected, is_true()) + expect_true(contentMatchExpected) #2 calls because Collection$new calls getFileListing once expect_that(fakeREST$getCollectionContentCallCount, equals(2)) @@ -237,7 +237,7 @@ test_that("get returns arvados file or subcollection from internal tree structur fish <- collection$get("animal/fish") fishIsNotNull <- !is.null(fish) - expect_that(fishIsNotNull, is_true()) + expect_true(fishIsNotNull) expect_that(fish$getName(), equals("fish")) }) @@ -256,8 +256,8 @@ test_that(paste("copy copies content to a new location inside file tree", dogExistsOnOldLocation <- !is.null(collection$get("animal/dog")) dogExistsOnNewLocation <- !is.null(collection$get("dog")) - expect_that(dogExistsOnOldLocation, is_true()) - expect_that(dogExistsOnNewLocation, is_true()) + expect_true(dogExistsOnOldLocation) + expect_true(dogExistsOnNewLocation) expect_that(fakeREST$copyCallCount, equals(1)) }) diff --git a/sdk/R/tests/testthat/test-CollectionTree.R b/sdk/R/tests/testthat/test-CollectionTree.R index 1a3aefecd0..c4bf9a1da7 100644 --- a/sdk/R/tests/testthat/test-CollectionTree.R +++ b/sdk/R/tests/testthat/test-CollectionTree.R @@ -34,16 +34,16 @@ test_that("constructor creates file tree from character array properly", { boat$getCollection() == "myCollection" expect_that(root$getName(), equals("")) - expect_that(rootIsOfTypeSubcollection, is_true()) - expect_that(rootHasNoParent, is_true()) - expect_that(animalIsOfTypeSubcollection, is_true()) - expect_that(animalsParentIsRoot, is_true()) - expect_that(animalContainsDog, is_true()) - expect_that(dogIsOfTypeArvadosFile, is_true()) - expect_that(dogsParentIsAnimal, is_true()) - expect_that(boatIsOfTypeArvadosFile, is_true()) - expect_that(boatsParentIsRoot, is_true()) - expect_that(allElementsBelongToSameCollection, is_true()) + expect_true(rootIsOfTypeSubcollection) + expect_true(rootHasNoParent) + expect_true(animalIsOfTypeSubcollection) + expect_true(animalsParentIsRoot) + expect_true(animalContainsDog) + expect_true(dogIsOfTypeArvadosFile) + expect_true(dogsParentIsAnimal) + expect_true(boatIsOfTypeArvadosFile) + expect_true(boatsParentIsRoot) + expect_true(allElementsBelongToSameCollection) }) test_that("getElement returns element from tree if element exists on specified path", { @@ -72,7 +72,7 @@ test_that("getElement returns NULL from tree if element doesn't exists on specif fish <- collectionTree$getElement("animal/fish") fishIsNULL <- is.null(fish) - expect_that(fishIsNULL, is_true()) + expect_true(fishIsNULL) }) test_that("getElement trims ./ from start of relativePath", { diff --git a/sdk/R/tests/testthat/test-HttpParser.R b/sdk/R/tests/testthat/test-HttpParser.R index 82c0fb0dd2..fb9f379b36 100644 --- a/sdk/R/tests/testthat/test-HttpParser.R +++ b/sdk/R/tests/testthat/test-HttpParser.R @@ -18,7 +18,7 @@ test_that("parseJSONResponse generates and returns JSON object from server respo result <- parser$parseJSONResponse(serverResponse) barExists <- !is.null(result$bar) - expect_that(barExists, is_true()) + expect_true(barExists) expect_that(unlist(result$bar$foo), equals(10)) }) @@ -40,7 +40,7 @@ test_that(paste("parseResponse generates and returns character vector", webDAVResponseSample = paste0("/c=aaaaa-bbbbb-ccccccccccccccc/c=aaaaa-bbbbb-ccccccccccccccc/Fri, 11 Jan 2018 1", "1:11:11 GMT