diff --git a/DESCRIPTION b/DESCRIPTION index a577330..74630c7 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: pipr Title: Client for the Poverty and Inequality Platform ('PIP') API -Version: 1.0.0 +Version: 1.1.0 Authors@R: c(person(given = "Tony", family = "Fujs", @@ -41,7 +41,6 @@ Suggests: rmarkdown, markdown, callr, - mockery, ggplot2, tidyr, ggthemes, @@ -53,17 +52,17 @@ Language: en-US Imports: attempt, curl, - httr, jsonlite, tibble, purrr, - memoise, - cachem, data.table, cli, rlang, - utils + utils, + httr2, + stringr, + vroom Depends: - R (>= 3.6.0) + R (>= 4.1.0) Config/testthat/edition: 3 Date: 2023-04-28 diff --git a/NAMESPACE b/NAMESPACE index 9a730e2..36d4135 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,8 +2,10 @@ export(call_aux) export(check_api) +export(delete_cache) export(display_aux) export(get_aux) +export(get_cache_info) export(get_pip_info) export(get_stats) export(get_versions) diff --git a/NEWS.md b/NEWS.md index a3798ea..901afe3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,13 @@ +# pipr 1.1.0 + +* [Use httr2](https://github.com/worldbank/pipr/pull/70) + * API responses are now cached locally according to the PIP API cache policy + from the PIP API responses headers + * `pipr` automatically handles retries when hitting the PIP API rate limiting + threshold + * Improved translation of HTTP errors into R error messages +* New helper functions `delete_cache()` and `get_cache_info()` + # pipr 1.0.0 * [Mock live API calls or skip them on CRAN](https://github.com/worldbank/pipr/pull/45) diff --git a/R/build_request.R b/R/build_request.R new file mode 100644 index 0000000..475e268 --- /dev/null +++ b/R/build_request.R @@ -0,0 +1,45 @@ +#' build_request +#' +#' @param server character: Server. For WB internal use only +#' @param api_version character: API version +#' @param endpoint character: PIP API endpoint +#' @param ... +#' +#' @return httr2 request +#' +build_request <- function(server, + api_version, + endpoint, + ...) { + + base_url <- select_base_url(server = server) + + params <- list(...) + params <- lapply(params, fix_params) + + req <- httr2::request(base_url) |> + httr2::req_url_path_append(api_version) |> + httr2::req_url_path_append(endpoint) |> + httr2::req_url_query(!!!params) |> + httr2::req_cache(tools::R_user_dir("pipr", which = "cache"), + use_on_error = TRUE, + debug = TRUE) |> + httr2::req_user_agent(pipr_user_agent) |> + httr2::req_error(body = parse_error_body) |> + httr2::req_retry( + is_transient = pip_is_transient, + after = retry_after, + max_seconds = 60 + ) + + return(req) + +} + +fix_params <- function(param) { + if (length(param) > 1) { + return(paste(param, collapse = ",")) + } else { + return(param) + } +} diff --git a/R/display_aux.R b/R/display_aux.R index d8e9105..59e288e 100644 --- a/R/display_aux.R +++ b/R/display_aux.R @@ -31,9 +31,12 @@ display_aux <- function(version = NULL, # ____________________________________________________________________________ # Build query string #### + req <- build_request(server = server, + api_version = api_version, + endpoint = "aux") + res <- req |> + httr2::req_perform() - u <- build_url(server, "aux", api_version = api_version) - res <- httr::GET(u) tbs_tb <- parse_response(res, simplify = simplify) tbs <- tbs_tb[["tables"]] if (isTRUE(run_cli)) { diff --git a/R/get_aux.R b/R/get_aux.R index ae70c65..85633b4 100644 --- a/R/get_aux.R +++ b/R/get_aux.R @@ -61,12 +61,15 @@ get_aux <- function(table = NULL, format <- match.arg(format) run_cli <- run_cli() # Build query string - u <- build_url(server, "aux", api_version = api_version) + req <- build_request(server = server, + api_version = api_version, + endpoint = "aux") # Return response # If no table is specified, returns list of available tables if (is.null(table)) { - res <- httr::GET(u) + res <- req |> + httr2::req_perform() tables <- parse_response(res, simplify = simplify) cli::cli_text("Auxiliary tables available are") cli::cli_ul(tables$tables) @@ -80,12 +83,15 @@ get_aux <- function(table = NULL, return(invisible(tables)) # If a table is specified, returns that table } else { - args <- build_args(.table = table, - .version = version, - .ppp_version = ppp_version, - .release_version = release_version, - .format = format) - res <- httr::GET(u, query = args, httr::user_agent(pipr_user_agent)) + req <- build_request(server = server, + api_version = api_version, + endpoint = "aux", + table = table, + version = version, + release_version = release_version, + format = format) + + res <- httr2::req_perform(req) rt <- parse_response(res, simplify = simplify) } diff --git a/R/get_stats.R b/R/get_stats.R index f350cf3..34319fe 100644 --- a/R/get_stats.R +++ b/R/get_stats.R @@ -69,7 +69,7 @@ get_stats <- function(country = "all", ppp_version = NULL, release_version = NULL, api_version = "v1", - format = c("rds", "json", "csv"), + format = c("arrow", "rds", "json", "csv"), simplify = TRUE, server = NULL) { # Match args @@ -95,23 +95,26 @@ get_stats <- function(country = "all", } # Build query string - args <- build_args( - .country = country, - .year = year, - .povline = povline, - .popshare = popshare, - .fill_gaps = fill_gaps, - .group_by = group_by, - .welfare_type = welfare_type, - .reporting_level = reporting_level, - .version = version, - .ppp_version = ppp_version, - .release_version = release_version, - .format = format + req <- build_request( + country = country, + year = year, + povline = povline, + popshare = popshare, + fill_gaps = fill_gaps, + group_by = group_by, + welfare_type = welfare_type, + reporting_level = reporting_level, + version = version, + ppp_version = ppp_version, + release_version = release_version, + format = format, + server = server, + api_version = api_version, + endpoint = endpoint ) - u <- build_url(server, endpoint, api_version) - # Send query - res <- httr::GET(u, query = args, httr::user_agent(pipr_user_agent)) + # Perform request + res <- req |> + httr2::req_perform() # Parse result out <- parse_response(res, simplify) @@ -136,20 +139,21 @@ get_wb <- function(year = "all", format <- match.arg(format) # Build query string - args <- build_args( - .country = "all", - .year = year, - .povline = povline, - .group_by = "wb", - .version = version, - .ppp_version = ppp_version, - .release_version = release_version, - .format = format + req <- build_request( + year = year, + povline = povline, + group_by = "wb", + version = version, + ppp_version = ppp_version, + release_version = release_version, + format = format, + server = server, + api_version = api_version, + endpoint = "pip-grp" ) - u <- build_url(server, "pip-grp", api_version) - - # Send query - res <- httr::GET(u, query = args, httr::user_agent(pipr_user_agent)) + # Perform request + res <- req |> + httr2::req_perform() # Parse result out <- parse_response(res, simplify) diff --git a/R/other.R b/R/other.R index ba9d3e6..a4952db 100644 --- a/R/other.R +++ b/R/other.R @@ -24,8 +24,10 @@ check_api <- function(api_version = "v1", server = NULL) { #' } get_versions <- function(api_version = "v1", server = NULL, simplify = TRUE) { check_internet() - u <- build_url(server, "versions", api_version) - res <- httr::GET(u, httr::user_agent(pipr_user_agent)) + req <- build_request(server = server, + api_version = api_version, + endpoint = "versions") + res <- httr2::req_perform(req) parse_response(res, simplify = simplify) } @@ -41,7 +43,9 @@ get_versions <- function(api_version = "v1", server = NULL, simplify = TRUE) { #' } get_pip_info <- function(api_version = "v1", server = NULL) { check_internet() - u <- build_url(server, "pip-info", api_version) - res <- httr::GET(u, httr::user_agent(pipr_user_agent)) + req <- build_request(server = server, + api_version = api_version, + endpoint = "pip-info") + res <- httr2::req_perform(req) parse_response(res, simplify = FALSE)$content } diff --git a/R/utils.R b/R/utils.R index 6df7e68..c7b0541 100644 --- a/R/utils.R +++ b/R/utils.R @@ -8,11 +8,13 @@ check_internet <- function() { #' health_check #' @inheritParams check_api #' @noRd -health_check <- function(api_version, server = NULL) { - u <- build_url(server, "health-check", api_version) - res <- httr::GET(u) +health_check <- function(api_version = "v1", server = NULL) { + req <- build_request(server = server, + api_version = api_version, + endpoint = "health-check") + res <- httr2::req_perform(req) attempt::stop_if_not( - .x = httr::status_code(res), + .x = httr2::resp_status(res), .p = ~ .x == 200, msg = "Could not connect to the API" ) @@ -23,22 +25,15 @@ health_check <- function(api_version, server = NULL) { #' @param res A httr response #' @param parsed A parsed response #' @noRd -check_status <- function(res, parsed) { - if (res$status_code != 200) { - if ("error" %in% names(parsed)) { +check_status <- function(res) { + if (httr2::resp_is_error(res)) { msg1 <- paste( - httr::http_status(res$status_code)$message, - parsed$error, + httr2::resp_status_desc(res), "Use simplify = FALSE to see the full error response.", sep = "\n*\t") - } else { - msg1 <- paste( - httr::http_status(res$status_code)$message, - "Use simplify = FALSE to see the full error response.", - sep = "\n*\t") - } + attempt::stop_if_not( - .x = httr::status_code(res), + .x = httr2::resp_status(res), .p = ~ .x == 200, msg = msg1 ) @@ -46,13 +41,13 @@ check_status <- function(res, parsed) { invisible(TRUE) } -#' build_url +#' build_base_url #' @param server character: Server #' @param endpoint character: Endpoint #' @param api_version character: API version #' @inheritParams get_stats #' @noRd -build_url <- function(server, endpoint, api_version) { +build_base_url <- function(server, endpoint, api_version) { base_url <- select_base_url(server = server) sprintf("%s/%s/%s", base_url, api_version, endpoint) } @@ -113,23 +108,29 @@ build_args <- function(.country = NULL, parse_response <- function(res, simplify) { # Get response type - type <- tryCatch(suppressWarnings(httr::http_type(res)), error = function(e) NULL) + type <- tryCatch(suppressWarnings(httr2::resp_content_type(res)), error = function(e) NULL) # Stop if response type is unknown attempt::stop_if(is.null(type), msg = "Invalid response format") + if (type == "application/vnd.apache.arrow.file") { + parsed <- arrow::read_feather(res$body) + } + if (type == "application/json") { - parsed <- jsonlite::fromJSON(httr::content(res, "text", encoding = "UTF-8")) + parsed <- jsonlite::fromJSON(httr2::resp_body_string(res, encoding = "UTF-8")) } if (type == "text/csv") { - parsed <- suppressMessages(httr::content(res, encoding = "UTF-8")) + parsed <- suppressMessages(vroom::vroom( + I(httr2::resp_body_string(res, encoding = "UTF-8"))) + ) } if (type == "application/rds") { - parsed <- unserialize(res$content) + parsed <- unserialize(res$body) } if (simplify) { - check_status(res, parsed) + httr2::resp_check_status(res, info = parsed$message) parsed <- tibble::as_tibble(parsed) # TEMP fix for renaming of columns # To be removed when pipapi#207 @@ -193,3 +194,127 @@ tmp_rename_cols <- function(df, url = "") { return(df) } + +#' pip_is_transient +#' +#' Helper function to determine if an error is due to the number of requests +#' going over the rate limit +#' +#' @param resp +#' +#' @return logical +#' +pip_is_transient <- function(resp) { + if (httr2::resp_is_error(resp)) { + if (httr2::resp_status(resp) == 429) { + stringr::str_detect(httr2::resp_body_json(resp, check_type = FALSE)$message, + "Rate limit is exceeded") + } else { + FALSE + } + } else { + FALSE + } +} + +#' retry_after +#' +#' Helper function to determine how much time to wait before a new +#' query can be sent +#' +#' @param resp +#' +#' @return numeric +#' +retry_after <- function(resp) { + if (httr2::resp_is_error(resp)) { + time <- httr2::resp_body_json(resp, check_type = FALSE)$message + time <- stringr::str_remove(time, "Rate limit is exceeded. Try again in ") + readr::parse_number(time) + } else { + 0 + } +} + +#' parse_error_body +#' +#' Helper function to parse error messages generated by the PIP API +#' +#' @param resp +#' +#' @return character +#' +parse_error_body <- function(resp) { + if (httr2::resp_is_error(resp)) { + if (is_gateway_timeout(resp)) { + # Handle gateway timeout + return(httr2::resp_status_desc(resp)) + } else if (is_bad_gateway(resp)) { + # Handle bad gateway timeout + return(httr2::resp_status_desc(resp)) + } else { + # Handle regular PIP errors + out <- httr2::resp_body_json(resp) + message1 <- out$error[[1]] + message2 <- out$details[[1]]$msg[[1]] + message3 <- paste(unlist(out$details[[names(out$details)]]$valid), collapse = ", ") + message <- c(message1, message2, message3) + return(message) + } + } +} + +is_gateway_timeout <- function(resp) { + httr2::resp_status(resp) == 504 & + httr2::resp_status_desc(resp) == "Gateway Timeout" +} + +is_bad_gateway <- function(resp) { + httr2::resp_status(resp) == 502 & + httr2::resp_status_desc(resp) == "Bad Gateway" +} + +#' Deletes content of the cache folder +#' +#' +#' @return Side effect. Deletes files. +#' +#' @export +#' +#' @examples \dontrun{delete_cache()} +delete_cache <- function() { + + cached_files <- list.files(tools::R_user_dir("pipr", which = "cache"), + full.names = TRUE) + + if (length(cached_files) == 0) { + message("Cache is empty. Nothing to delete") + } else { + lapply(cached_files, file.remove) + message("All items have been deleted from the cache.") + } +} + +#' Provides some information about cached items +#' +#' +#' @return character. +#' +#' @export +#' +#' @examples +#' \dontrun{get_cache_info()} +get_cache_info <- function() { + + cache_path <- tools::R_user_dir("pipr", which = "cache") + n_cached <- length(list.files(cache_path)) + + if (n_cached > 1) { + message_text <- " API responses are currently cached in " + } else { + message_text <- " API response is currently cached in " + } + + message(cli::format_message(c("Cache status:", + "i" = paste0(n_cached, message_text, cache_path)))) +} diff --git a/R/zzz.R b/R/zzz.R index f6a0d22..1387f4a 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -1,23 +1,23 @@ -.onLoad <- function(libname, pkgname) { - if (!Sys.getenv("PIPR_DISABLE_CACHING") == "TRUE") { - # d <- rappdirs::user_cache_dir("pipr") - # cm <- cachem::cache_disk(d, - # evict = "lru", - # max_size = 512 * 1024^2) - cm <- cachem::cache_mem(max_size = 512 * 1024^2, evict = "lru") - get_stats <<- memoise::memoise(get_stats, cache = cm) - get_wb <<- memoise::memoise(get_wb, cache = cm) - get_aux <<- memoise::memoise(get_aux, cache = cm) - get_versions <<- memoise::memoise(get_versions, cache = cm) - } - - options(cli.ignore_unknown_rstudio_theme = TRUE) - -} - -.onAttach <- function(libname, pkgname) { - if (!Sys.getenv("PIPR_DISABLE_CACHING") == "TRUE") { - packageStartupMessage("Info: Session based caching is enabled.") - } -} +# .onLoad <- function(libname, pkgname) { +# if (!Sys.getenv("PIPR_DISABLE_CACHING") == "TRUE") { +# # d <- rappdirs::user_cache_dir("pipr") +# # cm <- cachem::cache_disk(d, +# # evict = "lru", +# # max_size = 512 * 1024^2) +# cm <- cachem::cache_mem(max_size = 512 * 1024^2, evict = "lru") +# get_stats <<- memoise::memoise(get_stats, cache = cm) +# get_wb <<- memoise::memoise(get_wb, cache = cm) +# get_aux <<- memoise::memoise(get_aux, cache = cm) +# get_versions <<- memoise::memoise(get_versions, cache = cm) +# } +# +# options(cli.ignore_unknown_rstudio_theme = TRUE) +# +# } +# +# .onAttach <- function(libname, pkgname) { +# if (!Sys.getenv("PIPR_DISABLE_CACHING") == "TRUE") { +# packageStartupMessage("Info: Session based caching is enabled.") +# } +# } diff --git a/man/build_request.Rd b/man/build_request.Rd new file mode 100644 index 0000000..0dfd41b --- /dev/null +++ b/man/build_request.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/build_request.R +\name{build_request} +\alias{build_request} +\title{build_request} +\usage{ +build_request(server, api_version, endpoint, ...) +} +\arguments{ +\item{server}{character: Server. For WB internal use only} + +\item{api_version}{character: API version} + +\item{endpoint}{character: PIP API endpoint} + +\item{...}{} +} +\value{ +httr2 request +} +\description{ +build_request +} diff --git a/man/delete_cache.Rd b/man/delete_cache.Rd new file mode 100644 index 0000000..7f004bd --- /dev/null +++ b/man/delete_cache.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{delete_cache} +\alias{delete_cache} +\title{Deletes content of the cache folder} +\usage{ +delete_cache() +} +\value{ +Side effect. Deletes files. +} +\description{ +Deletes content of the cache folder +} +\examples{ +\dontrun{delete_cache()} +} diff --git a/man/get_cache_info.Rd b/man/get_cache_info.Rd new file mode 100644 index 0000000..e09c361 --- /dev/null +++ b/man/get_cache_info.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{get_cache_info} +\alias{get_cache_info} +\title{Provides some information about cached items} +\usage{ +get_cache_info() +} +\value{ +character. +} +\description{ +Provides some information about cached items +} +\examples{ +\dontrun{get_cache_info()} +} diff --git a/man/get_stats.Rd b/man/get_stats.Rd index 921f9f9..9752fbc 100644 --- a/man/get_stats.Rd +++ b/man/get_stats.Rd @@ -18,7 +18,7 @@ get_stats( ppp_version = NULL, release_version = NULL, api_version = "v1", - format = c("rds", "json", "csv"), + format = c("arrow", "rds", "json", "csv"), simplify = TRUE, server = NULL ) diff --git a/man/parse_error_body.Rd b/man/parse_error_body.Rd new file mode 100644 index 0000000..f8be1fd --- /dev/null +++ b/man/parse_error_body.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{parse_error_body} +\alias{parse_error_body} +\title{parse_error_body} +\usage{ +parse_error_body(resp) +} +\arguments{ +\item{resp}{} +} +\value{ +character +} +\description{ +Helper function to parse error messages generated by the PIP API +} diff --git a/man/pip_is_transient.Rd b/man/pip_is_transient.Rd new file mode 100644 index 0000000..83f58bf --- /dev/null +++ b/man/pip_is_transient.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{pip_is_transient} +\alias{pip_is_transient} +\title{pip_is_transient} +\usage{ +pip_is_transient(resp) +} +\arguments{ +\item{resp}{} +} +\value{ +logical +} +\description{ +Helper function to determine if an error is due to the number of requests +going over the rate limit +} diff --git a/man/retry_after.Rd b/man/retry_after.Rd new file mode 100644 index 0000000..49bd284 --- /dev/null +++ b/man/retry_after.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{retry_after} +\alias{retry_after} +\title{retry_after} +\usage{ +retry_after(resp) +} +\arguments{ +\item{resp}{} +} +\value{ +numeric +} +\description{ +Helper function to determine how much time to wait before a new +query can be sent +} diff --git a/tests/testthat/test-caching.R b/tests/testthat/test-caching.R index ca55a93..0275c0b 100644 --- a/tests/testthat/test-caching.R +++ b/tests/testthat/test-caching.R @@ -1,33 +1,33 @@ -library(callr) - -test_that("Caching is enabled by default", { - skip_on_cran() - # Setup external R session - r <- callr::r_session$new(options = callr::r_session_options(user_profile = FALSE)) - r$run(function() Sys.setenv("PIPR_DISABLE_CACHING" = "FALSE")) - r$run(function() library(pipr)) - # Check that main functions are cached - tmp <- r$run(function() memoise::is.memoised(get_stats)) - expect_true(tmp) - tmp <- r$run(function() memoise::is.memoised(get_wb)) - expect_true(tmp) - tmp <- r$run(function() memoise::is.memoised(get_aux)) - expect_true(tmp) - r$kill() -}) - -test_that("Caching can be disabled", { - skip_on_cran() - # Setup external R session - r <- callr::r_session$new(options = callr::r_session_options(user_profile = FALSE)) - r$run(function() Sys.setenv("PIPR_DISABLE_CACHING" = "TRUE")) - r$run(function() library(pipr)) - # Check that main functions are NOT cached - tmp <- r$run(function() memoise::is.memoised(get_stats)) - expect_false(tmp) - tmp <- r$run(function() memoise::is.memoised(get_wb)) - expect_false(tmp) - tmp <- r$run(function() memoise::is.memoised(get_aux)) - expect_false(tmp) - r$kill() -}) +# library(callr) +# +# test_that("Caching is enabled by default", { +# skip_on_cran() +# # Setup external R session +# r <- callr::r_session$new(options = callr::r_session_options(user_profile = FALSE)) +# r$run(function() Sys.setenv("PIPR_DISABLE_CACHING" = "FALSE")) +# r$run(function() library(pipr)) +# # Check that main functions are cached +# tmp <- r$run(function() memoise::is.memoised(get_stats)) +# expect_true(tmp) +# tmp <- r$run(function() memoise::is.memoised(get_wb)) +# expect_true(tmp) +# tmp <- r$run(function() memoise::is.memoised(get_aux)) +# expect_true(tmp) +# r$kill() +# }) +# +# test_that("Caching can be disabled", { +# skip_on_cran() +# # Setup external R session +# r <- callr::r_session$new(options = callr::r_session_options(user_profile = FALSE)) +# r$run(function() Sys.setenv("PIPR_DISABLE_CACHING" = "TRUE")) +# r$run(function() library(pipr)) +# # Check that main functions are NOT cached +# tmp <- r$run(function() memoise::is.memoised(get_stats)) +# expect_false(tmp) +# tmp <- r$run(function() memoise::is.memoised(get_wb)) +# expect_false(tmp) +# tmp <- r$run(function() memoise::is.memoised(get_aux)) +# expect_false(tmp) +# r$kill() +# }) diff --git a/tests/testthat/test-display_aux.R b/tests/testthat/test-display_aux.R index c6e6fb8..c136947 100644 --- a/tests/testthat/test-display_aux.R +++ b/tests/testthat/test-display_aux.R @@ -4,8 +4,10 @@ test_that("returns proper table", { api_version <- "v1" simplify <- TRUE server <- NULL - u <- build_url(server, "aux", api_version = api_version) - res <- httr::GET(u) + req <- build_request(server = server, + api_version = api_version, + endpoint = "aux") + res <- httr2::req_perform(req) tbs_tb <- parse_response(res, simplify = simplify) tt <- suppressMessages(display_aux(server = server, simplify = simplify, api_version = api_version)) diff --git a/tests/testthat/test-get_aux.R b/tests/testthat/test-get_aux.R index eee97b0..ad9f14a 100644 --- a/tests/testthat/test-get_aux.R +++ b/tests/testthat/test-get_aux.R @@ -38,30 +38,14 @@ test_that("get_aux() works when calling specific tables", { expect_true(tibble::is_tibble(res)) # Check failure if table doesn't exist - # TO DO: Use prod server for this test when API has been released - # expect_error(get_aux("tmp")) - # expect_true(is.list(get_aux("tmp", simplify = FALSE))) skip_if(Sys.getenv("PIPR_RUN_LOCAL_TESTS") != "TRUE") expect_error(get_aux("wrong-table-name", server = "qa")) - expect_true(is.list(get_aux("wrong-table-name", simplify = FALSE, server = "prod"))) # Check all tables - skip("survey_metadata gives a 500 error. Need to add functionality for list data") - dl <- lapply(res, function(x) try(get_aux(x))) + # skip("survey_metadata gives a 500 error. Need to add functionality for list data") + dl <- lapply(res$tables, function(x) try(get_aux(x))) expect_true(all(sapply(dl, tibble::is_tibble))) - expect_true(sapply(dl, function(x) any(class(x) != "try-error"))) - # expect_false(sapply(dl, function(x) any(names(x) == "error"))) -}) - -test_that("User agent works", { - skip_if_offline() - skip_on_cran() - # res <- get_aux(simplify = FALSE) - # tmp <- res$response$request$options$useragent - # expect_identical(tmp, pipr_user_agent) - res <- get_aux("gdp", simplify = FALSE) - tmp <- res$response$request$options$useragent - expect_identical(tmp, pipr_user_agent) + expect_true(all(sapply(dl, function(x) any(class(x) != "try-error")))) }) ## Test helper functions ---- @@ -82,84 +66,3 @@ test_that("get_regions() works", { expect_true(tibble::is_tibble(res)) expect_identical(res, res2) }) - - -# test_that("get_countries() with mocking works", { -# mockery::stub(get_aux, "httr::GET", function(...) { -# readRDS(test_path("testdata", "response-country.RDS")) -# }) -# -# mockery::stub(get_countries, "get_aux", function(...) { -# readRDS(test_path("testdata", "response-country.RDS")) -# }) -# -# res1 <- get_aux("countries") -# res2 <- parse_response(get_countries(), TRUE) -# -# expect_true(tibble::is_tibble(res1)) -# expect_true(tibble::is_tibble(res2)) -# -# expect_equal(dim(res1), dim(res2)) -# expect_identical(res1, res2) -# }) -# -# -# test_that("get_regions() with mocking works", { -# mockery::stub(get_aux, "httr::GET", function(...) { -# readRDS(test_path("testdata", "response-regions.RDS")) -# }) -# -# mockery::stub(get_regions, "get_aux", function(...) { -# readRDS(test_path("testdata", "response-regions.RDS")) -# }) -# -# res1 <- get_aux("regions") -# res2 <- parse_response(get_regions(), TRUE) -# -# expect_true(tibble::is_tibble(res1)) -# expect_true(tibble::is_tibble(res2)) -# -# expect_equal(dim(res1), dim(res2)) -# expect_identical(res1, res2) -# }) -# -# -# test_that("get_cpi() with mocking works", { -# mockery::stub(get_aux, "httr::GET", function(...) { -# readRDS(test_path("testdata", "response-cpi.RDS")) -# }) -# -# mockery::stub(get_cpi, "get_aux", function(...) { -# readRDS(test_path("testdata", "response-cpi.RDS")) -# }) -# -# res1 <- get_aux("cpi") -# res2 <- parse_response(get_cpi(), TRUE) -# -# expect_true(tibble::is_tibble(res1)) -# expect_true(tibble::is_tibble(res2)) -# -# expect_equal(dim(res1), dim(res2)) -# expect_identical(res1, res2) -# }) -# -# test_that("get_dictionary() with mocking works", { -# #Waiting for this PR to be merged https://github.com/worldbank/pipr/pull/43 -# #so that get_dictionary() works -# mockery::stub(get_aux, "httr::GET", function(...) { -# readRDS(test_path("testdata", "response-dictionary.RDS")) -# }) -# -# mockery::stub(get_dictionary, "get_aux", function(...) { -# readRDS(test_path("testdata", "response-dictionary.RDS")) -# }) -# -# res1 <- get_aux("dictionary") -# res2 <- parse_response(get_dictionary(), TRUE) -# -# expect_true(tibble::is_tibble(res1)) -# expect_true(tibble::is_tibble(res2)) -# -# expect_equal(dim(res1), dim(res2)) -# expect_identical(res1, res2) -# }) diff --git a/tests/testthat/test-get_stats.R b/tests/testthat/test-get_stats.R index 2ee287f..3c8da10 100644 --- a/tests/testthat/test-get_stats.R +++ b/tests/testthat/test-get_stats.R @@ -185,16 +185,3 @@ test_that("get_wb() works w/ simplify = FALSE", { expect_true(is.data.frame(res$content)) expect_gte(nrow(res$content), 3) }) - - -test_that("User agent works", { - skip_if_offline() - skip_on_cran() - res <- get_stats("AGO", 2000, simplify = FALSE) - tmp <- res$response$request$options$useragent - expect_identical(tmp, pipr_user_agent) - - res <- get_wb(2000, simplify = FALSE) - tmp <- res$response$request$options$useragent - expect_identical(tmp, pipr_user_agent) -}) diff --git a/tests/testthat/test-other.R b/tests/testthat/test-other.R index 98894eb..a55bc90 100644 --- a/tests/testthat/test-other.R +++ b/tests/testthat/test-other.R @@ -5,15 +5,15 @@ test_that("health_check() works", { skip_if_offline() skip_on_cran() res <- health_check(api_version = "v1") - expect_identical(httr::content(res)[[1]], "PIP API is running") + expect_identical(httr2::resp_body_json(res)[[1]], "PIP API is running") expect_equal(res$status_code, 200) expect_invisible(health_check(api_version = "v1")) expect_error(health_check("xx")) skip_if(Sys.getenv("PIPR_RUN_LOCAL_TESTS") != "TRUE") skip_if(is.null(curl::nslookup(dev_host, error = FALSE)), message = "Could not connect to DEV host") - expect_identical(httr::content(health_check(api_version = "v1", server = "dev"))[[1]], "PIP API is running") + expect_identical(httr2::resp_body_json(health_check(api_version = "v1", server = "dev"))[[1]], "PIP API is running") skip_if(is.null(curl::nslookup(qa_host, error = FALSE)), message = "Could not connect to QA host") - expect_identical(httr::content(health_check(api_version = "v1", server = "qa"))[[1]], "PIP API is running") + expect_identical(httr2::resp_body_json(health_check(api_version = "v1", server = "qa"))[[1]], "PIP API is running") }) test_that("get_versions() works", { diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 8ff7f74..a735fe4 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -17,7 +17,7 @@ test_that("check_internet() works", { test_that("check_api() works", { skip_if_offline() skip_on_cran() - res <- check_api("v1", server = NULL) + res <- check_api("v1", server = "qa") expect_equal(res, "PIP API is running") }) @@ -26,68 +26,63 @@ test_that("check_status() works", { skip_on_cran() # 200 res <- health_check("v1") - parsed <- parse_response(res, simplify = FALSE)$content - expect_true(check_status(res, parsed)) + expect_true(check_status(res)) # 404 res <- res_ex_404 - parsed <- parse_response(res, simplify = FALSE)$content - expect_error(check_status(res, parsed)) + expect_error(check_status(res)) # 500 res <- res_ex_404 - parsed <- parse_response(res, simplify = FALSE)$content res$status_code <- 500 - parsed$error <- NULL - parsed$details <- NULL - expect_error(check_status(res, parsed)) + expect_error(check_status(res)) }) -test_that("build_url() works", { +test_that("build_base_url() works", { # Check that url is correctly pasted together - x <- build_url(server = NULL, endpoint = "pip", api_version = "v1") + x <- build_base_url(server = NULL, endpoint = "pip", api_version = "v1") expect_identical(x, paste0(prod_url, "/v1/pip")) - x <- build_url("prod", "pip", api_version = "v1") + x <- build_base_url("prod", "pip", api_version = "v1") expect_identical(x, paste0(prod_url, "/v1/pip")) - x <- build_url("prod", "pip-grp", api_version = "v2") + x <- build_base_url("prod", "pip-grp", api_version = "v2") expect_identical(x, paste0(prod_url, "/v2/pip-grp")) # Expect error if server arg is incorrect - expect_error(build_url("tmp", "pip", "v1")) + expect_error(build_base_url("tmp", "pip", "v1")) # Check internal URLs skip_if(Sys.getenv("PIPR_RUN_LOCAL_TESTS") != "TRUE") - x <- build_url("qa", "pip", "v1") + x <- build_base_url("qa", "pip", "v1") expect_identical(x, paste0(Sys.getenv("PIP_QA_URL"), "/v1/pip")) - x <- build_url("dev", "pip", "v1") + x <- build_base_url("dev", "pip", "v1") expect_identical(x, paste0(Sys.getenv("PIP_DEV_URL"), "/v1/pip")) # Expect error if ENV vars are not found skip_if(Sys.getenv("PIP_QA_URL") != "") - expect_error(build_url("qa", "pip", "v1")) + expect_error(build_base_url("qa", "pip", "v1")) skip_if(Sys.getenv("PIP_DEV_URL") != "") - expect_error(build_url("dev", "pip", "v1")) + expect_error(build_base_url("dev", "pip", "v1")) }) -test_that("build_url() works for internal URLS", { +test_that("build_base_url() works for internal URLS", { # Check internal URLs skip_if(Sys.getenv("PIPR_RUN_LOCAL_TESTS") != "TRUE") - x <- build_url("qa", "pip", "v1") + x <- build_base_url("qa", "pip", "v1") expect_identical(x, paste0(Sys.getenv("PIP_QA_URL"), "/v1/pip")) - x <- build_url("dev", "pip", "v1") + x <- build_base_url("dev", "pip", "v1") expect_identical(x, paste0(Sys.getenv("PIP_DEV_URL"), "/v1/pip")) }) -test_that("build_url() throws error for internal URLs if ENV vars are not found", { +test_that("build_base_url() throws error for internal URLs if ENV vars are not found", { # Expect error if ENV vars are not found skip_if(Sys.getenv("PIP_QA_URL") != "") - expect_error(build_url("qa", "pip", "v1")) + expect_error(build_base_url("qa", "pip", "v1")) skip_if(Sys.getenv("PIP_DEV_URL") != "") - expect_error(build_url("dev", "pip", "v1")) + expect_error(build_base_url("dev", "pip", "v1")) }) test_that("build_args() works for all individual parameters", { @@ -204,7 +199,7 @@ test_that("parse_response() works for different formats", { res <- parse_response(res_ex_json, simplify = FALSE) expect_identical(names(res), c("url", "status", "type", "content", "response")) expect_identical(class(res), "pip_api") - expect_identical(class(res$response), "response") + expect_identical(class(res$response), "httr2_response") expect_identical(class(res$content), "data.frame") # csv @@ -213,7 +208,7 @@ test_that("parse_response() works for different formats", { res <- parse_response(res_ex_csv, simplify = FALSE) expect_identical(names(res), c("url", "status", "type", "content", "response")) expect_identical(class(res), "pip_api") - expect_identical(class(res$response), "response") + expect_identical(class(res$response), "httr2_response") expect_true(all(class(res$content) %in% c("spec_tbl_df", "tbl_df", "tbl", "data.frame"))) # rds @@ -222,7 +217,7 @@ test_that("parse_response() works for different formats", { res <- parse_response(res_ex_rds, simplify = FALSE) expect_identical(names(res), c("url", "status", "type", "content", "response")) expect_identical(class(res), "pip_api") - expect_identical(class(res$response), "response") + expect_identical(class(res$response), "httr2_response") expect_true(all(class(res$content) %in% c("data.table", "data.frame"))) }) diff --git a/tests/testthat/testdata/res-ex-csv.RDS b/tests/testthat/testdata/res-ex-csv.RDS index f40b1e2..a445038 100644 Binary files a/tests/testthat/testdata/res-ex-csv.RDS and b/tests/testthat/testdata/res-ex-csv.RDS differ diff --git a/tests/testthat/testdata/res-ex-json.RDS b/tests/testthat/testdata/res-ex-json.RDS index 23ed9b3..868b576 100644 Binary files a/tests/testthat/testdata/res-ex-json.RDS and b/tests/testthat/testdata/res-ex-json.RDS differ diff --git a/tests/testthat/testdata/res-ex-rds.RDS b/tests/testthat/testdata/res-ex-rds.RDS index 6bbeed4..3ed461e 100644 Binary files a/tests/testthat/testdata/res-ex-rds.RDS and b/tests/testthat/testdata/res-ex-rds.RDS differ