diff --git a/NAMESPACE b/NAMESPACE index 972b467..012bb32 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -18,10 +18,6 @@ S3method(.as_nectar_tidy_policy,nectar_tidy_policy) S3method(resp_parse,default) S3method(resp_parse,httr2_response) S3method(resp_parse,list) -S3method(resp_tidy,default) -S3method(resp_tidy,httr2_response) -S3method(resp_tidy,list) -S3method(resp_tidy,nectar_responses) export(auth_api_key) export(auth_prepare) export(choose_pagination_fn) diff --git a/R/resp_parse.R b/R/resp_parse.R index 68420c6..cb0c799 100644 --- a/R/resp_parse.R +++ b/R/resp_parse.R @@ -41,7 +41,7 @@ resp_parse.default <- function( ) { .nectar_abort( c( - "{.arg {arg}} must be a {.cls list} or a {.cls httr2_response}.", + "{.arg {arg}} must be a {.cls httr2_response} or a {.cls list} of {.cls httr2_response} objects.", x = "{.arg {arg}} is {.obj_type_friendly {resps}}." ), subclass = "unsupported_response_class", @@ -58,6 +58,15 @@ resp_parse.httr2_response <- function(resps, ..., response_parser = resp_tidy) { #' @export resp_parse.list <- function(resps, ..., response_parser = resp_tidy) { + if (!purrr::every(resps, \(x) inherits(x, "httr2_response"))) { + .nectar_abort( + c( + "{.arg resps} must be a list of {.cls httr2_response} objects.", + x = "Not all elements of {.arg resps} are {.cls httr2_response} objects." + ), + subclass = "unsupported_response_class" + ) + } resps_parsed <- .resp_parse_impl(resps, response_parser, ...) .resps_combine(resps_parsed) } diff --git a/R/resp_tidy.R b/R/resp_tidy.R index 8ea7329..04ab610 100644 --- a/R/resp_tidy.R +++ b/R/resp_tidy.R @@ -7,17 +7,11 @@ #' #' @inheritParams .shared-params #' -#' @returns The extracted and cleaned response, or, for a list of responses, -#' those responses cleaned then concatenated via [httr2::resps_data()]. By -#' default, the response is processed with [resp_body_auto()]. +#' @returns The extracted and cleaned response, or `NULL` if `resp` is `NULL`. +#' By default, the response is processed with [resp_body_auto()]. If the +#' request includes a `resp_tidy` policy (set via [req_tidy_policy()]), that +#' policy's function and arguments are used instead. #' -#' @seealso [resp_tidy_json()] for an opinionated response parser for JSON -#' responses, [resp_body_auto()] (etc) for a family of response parsers that -#' attempts to automatically select the appropriate parser based on the -#' response content type, [httr2::resp_body_raw()] (etc) for the underlying -#' httr2 response parsers, and [resp_parse()] for an alternative approach to -#' dealing with responses (particularly useful if the request does not include -#' a `resp_tidy` policy). #' @family opinionated response parsers #' @export #' @@ -35,46 +29,20 @@ #' # fetched with httr2::req_perform() or req_perform_opinionated(). #' resp$request <- req #' resp_tidy(resp) -resp_tidy <- function(resps) { - UseMethod("resp_tidy") -} - -#' @export -resp_tidy.httr2_response <- function(resps) { - req <- httr2::resp_request(resps) +resp_tidy <- function(resp) { + if (is.null(resp)) { + return(NULL) + } + .check_httr2_response(resp) + req <- httr2::resp_request(resp) if (length(req$policies$resp_tidy)) { return( rlang::exec( req$policies$resp_tidy$tidy_fn, - resps, + resp, !!!req$policies$resp_tidy$tidy_args ) ) } - resp_body_auto(resps) -} - -#' @export -resp_tidy.nectar_responses <- function(resps) { - httr2::resps_data(resps, resp_tidy) -} - -#' @export -resp_tidy.list <- function(resps) { - if (length(resps) && inherits(resps[[1]], "httr2_response")) { - class(resps) <- c("nectar_responses", "list") - return(resp_tidy(resps)) - } - NextMethod() -} - -#' @export -resp_tidy.default <- function(resps) { - .nectar_abort( - c( - "No method is available to {.fn nectar::resp_tidy} this object.", - i = "{.fn nectar::resp_tidy} expects {.cls httr2_response} objects, or lists thereof." - ), - "unsupported_response_class" - ) + resp_body_auto(resp) } diff --git a/R/utils.R b/R/utils.R index e38f5eb..2088916 100644 --- a/R/utils.R +++ b/R/utils.R @@ -1,3 +1,26 @@ +# check_httr2_response --------------------------------------------------------- + +#' Check that an object is an httr2_response +#' +#' @param x An object to check. +#' @param call The calling environment for error reporting. +#' +#' @return `x`, if it passes the check. +#' @keywords internal +.check_httr2_response <- function(x, call = rlang::caller_env()) { + if (!inherits(x, "httr2_response")) { + .nectar_abort( + c( + "{.arg x} must be a {.cls httr2_response} object.", + x = "{.arg x} is {.obj_type_friendly {x}}." + ), + subclass = "not_httr2_response", + call = call + ) + } + x +} + # compact_nested_list ---------------------------------------------------------- #' Discard empty elements diff --git a/man/dot-check_httr2_response.Rd b/man/dot-check_httr2_response.Rd new file mode 100644 index 0000000..73db001 --- /dev/null +++ b/man/dot-check_httr2_response.Rd @@ -0,0 +1,20 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{.check_httr2_response} +\alias{.check_httr2_response} +\title{Check that an object is an httr2_response} +\usage{ +.check_httr2_response(x, call = rlang::caller_env()) +} +\arguments{ +\item{x}{An object to check.} + +\item{call}{The calling environment for error reporting.} +} +\value{ +\code{x}, if it passes the check. +} +\description{ +Check that an object is an httr2_response +} +\keyword{internal} diff --git a/man/resp_tidy.Rd b/man/resp_tidy.Rd index 96133a8..193f78b 100644 --- a/man/resp_tidy.Rd +++ b/man/resp_tidy.Rd @@ -4,18 +4,17 @@ \alias{resp_tidy} \title{Extract and clean an API response} \usage{ -resp_tidy(resps) +resp_tidy(resp) } \arguments{ -\item{resps}{(\code{httr2_response}, \code{nectar_responses}, or \code{list}) A single -\code{\link[httr2:response]{httr2::response()}} object (as returned by \code{\link[httr2:req_perform]{httr2::req_perform()}}) or a -list of such objects (as returned by \code{\link[=req_perform_opinionated]{req_perform_opinionated()}} or -\code{\link[httr2:req_perform_iterative]{httr2::req_perform_iterative()}}).} +\item{resp}{(\code{httr2_response}) A single \code{\link[httr2:response]{httr2::response()}} object (as +returned by \code{\link[httr2:req_perform]{httr2::req_perform()}}).} } \value{ -The extracted and cleaned response, or, for a list of responses, -those responses cleaned then concatenated via \code{\link[httr2:resps_data]{httr2::resps_data()}}. By -default, the response is processed with \code{\link[=resp_body_auto]{resp_body_auto()}}. +The extracted and cleaned response, or \code{NULL} if \code{resp} is \code{NULL}. +By default, the response is processed with \code{\link[=resp_body_auto]{resp_body_auto()}}. If the +request includes a \code{resp_tidy} policy (set via \code{\link[=req_tidy_policy]{req_tidy_policy()}}), that +policy's function and arguments are used instead. } \description{ API responses generally follow a structured format. Use this function to @@ -39,14 +38,6 @@ resp$request <- req resp_tidy(resp) } \seealso{ -\code{\link[=resp_tidy_json]{resp_tidy_json()}} for an opinionated response parser for JSON -responses, \code{\link[=resp_body_auto]{resp_body_auto()}} (etc) for a family of response parsers that -attempts to automatically select the appropriate parser based on the -response content type, \code{\link[httr2:resp_body_raw]{httr2::resp_body_raw()}} (etc) for the underlying -httr2 response parsers, and \code{\link[=resp_parse]{resp_parse()}} for an alternative approach to -dealing with responses (particularly useful if the request does not include -a \code{resp_tidy} policy). - Other opinionated response parsers: \code{\link[=req_tidy_policy]{req_tidy_policy()}}, \code{\link[=resp_tidy_json]{resp_tidy_json()}}, diff --git a/tests/testthat/_snaps/resp_parse.md b/tests/testthat/_snaps/resp_parse.md index 27e3f0c..ca8d7c8 100644 --- a/tests/testthat/_snaps/resp_parse.md +++ b/tests/testthat/_snaps/resp_parse.md @@ -6,6 +6,17 @@ Output Error: - ! `1` must be a or a . + ! `1` must be a or a of objects. x `1` is a number. +# resp_parse fails gracefully for list of non-responses (#88) + + Code + (expect_pkg_error_classes(resp_parse(list(1, 2)), "nectar", + "unsupported_response_class")) + Output + + Error in `resp_parse()`: + ! `resps` must be a list of objects. + x Not all elements of `resps` are objects. + diff --git a/tests/testthat/_snaps/resp_tidy.md b/tests/testthat/_snaps/resp_tidy.md index c6d35c1..cba3674 100644 --- a/tests/testthat/_snaps/resp_tidy.md +++ b/tests/testthat/_snaps/resp_tidy.md @@ -1,22 +1,10 @@ -# resp_tidy fails gracefully for non-responses (#40) +# resp_tidy errors for non-response input (#88) Code - (expect_pkg_error_classes(resp_tidy(test_obj), "nectar", - "unsupported_response_class")) + (expect_pkg_error_classes(resp_tidy(1), "nectar", "not_httr2_response")) Output - + Error in `resp_tidy()`: - ! No method is available to `nectar::resp_tidy()` this object. - i `nectar::resp_tidy()` expects objects, or lists thereof. - -# resp_tidy fails gracefully for lists of non-responses (#40) - - Code - (expect_pkg_error_classes(resp_tidy(test_obj), "nectar", - "unsupported_response_class")) - Output - - Error in `resp_tidy()`: - ! No method is available to `nectar::resp_tidy()` this object. - i `nectar::resp_tidy()` expects objects, or lists thereof. + ! `x` must be a object. + x `x` is a number. diff --git a/tests/testthat/_snaps/utils.md b/tests/testthat/_snaps/utils.md new file mode 100644 index 0000000..5c00664 --- /dev/null +++ b/tests/testthat/_snaps/utils.md @@ -0,0 +1,11 @@ +# .check_httr2_response errors with not_httr2_response for non-response (#noissue) + + Code + (expect_pkg_error_classes(.check_httr2_response(1), "nectar", + "not_httr2_response")) + Output + + Error: + ! `x` must be a object. + x `x` is a number. + diff --git a/tests/testthat/test-resp_parse.R b/tests/testthat/test-resp_parse.R index 204b7a5..79ac23d 100644 --- a/tests/testthat/test-resp_parse.R +++ b/tests/testthat/test-resp_parse.R @@ -2,13 +2,20 @@ test_that("resp_parse fails gracefully for unsupported classes (#40)", { expect_nectar_error_snapshot(resp_parse(1), "unsupported_response_class") }) -test_that("resp_parse parses json-containing httr2_response objects", { +test_that("resp_parse fails gracefully for list of non-responses (#88)", { + expect_nectar_error_snapshot( + resp_parse(list(1, 2)), + "unsupported_response_class" + ) +}) + +test_that("resp_parse parses json-containing httr2_response objects (#10)", { mock_response <- httr2::response_json(body = 1:3) test_result <- resp_parse(mock_response) expect_identical(test_result, as.list(1:3)) }) -test_that("resp_parse parses httr2_response objects with specified parser", { +test_that("resp_parse parses httr2_response objects with specified parser (#10)", { mock_response <- httr2::response_json(body = 1:3) parser <- function(resp) { unlist(httr2::resp_body_json(resp)) @@ -17,13 +24,13 @@ test_that("resp_parse parses httr2_response objects with specified parser", { expect_identical(test_result, 1:3) }) -test_that("resp_parse returns raw resp if NULL parser specified", { +test_that("resp_parse returns raw resp if NULL parser specified (#10)", { mock_response <- httr2::response_json(body = 1:3) test_result <- resp_parse(mock_response, response_parser = NULL) expect_identical(test_result, mock_response) }) -test_that("resp_parse accepts parser args", { +test_that("resp_parse accepts parser args (#10)", { mock_response <- httr2::response_json(body = 1:3) parser <- function(resp, unlist = FALSE) { x <- httr2::resp_body_json(resp) @@ -42,7 +49,7 @@ test_that("resp_parse accepts parser args", { expect_identical(test_result, 1:3) }) -test_that("resp_parse parses lists of httr2_responses", { +test_that("resp_parse parses lists of httr2_responses (#10)", { mock_response <- list( httr2::response_json(body = 1:3), httr2::response_json(body = 4:6) @@ -54,7 +61,7 @@ test_that("resp_parse parses lists of httr2_responses", { expect_identical(test_result, 1:6) }) -test_that("resp_parse works for raw results", { +test_that("resp_parse works for raw results (#11)", { # reqs <- list( # httr2::request("https://httr2.r-lib.org/logo.png"), # httr2::request("https://docs.ropensci.org/magick/logo.png") diff --git a/tests/testthat/test-resp_tidy.R b/tests/testthat/test-resp_tidy.R index e8c6cf1..afb455f 100644 --- a/tests/testthat/test-resp_tidy.R +++ b/tests/testthat/test-resp_tidy.R @@ -1,26 +1,21 @@ -test_that("resp_tidy fails gracefully for non-responses (#40)", { - test_obj <- 1 - expect_nectar_error_snapshot( - resp_tidy(test_obj), - "unsupported_response_class" - ) +test_that("resp_tidy returns NULL for NULL input (#88)", { + expect_null(resp_tidy(NULL)) }) -test_that("resp_tidy fails gracefully for lists of non-responses (#40)", { - test_obj <- list(a = letters, b = 1:26) +test_that("resp_tidy errors for non-response input (#88)", { expect_nectar_error_snapshot( - resp_tidy(test_obj), - "unsupported_response_class" + resp_tidy(1), + "not_httr2_response" ) }) -test_that("resp_tidy parses json-containing httr2_response objects (#40)", { +test_that("resp_tidy parses json-containing httr2_response objects (#40, #88)", { mock_response <- httr2::response_json(body = 1:3) test_result <- resp_tidy(mock_response) expect_identical(test_result, as.list(1:3)) }) -test_that("resp_tidy parses httr2_response objects with resp_tidy policy (#40)", { +test_that("resp_tidy parses httr2_response objects with resp_tidy policy (#40, #88)", { mock_response <- httr2::response_json(body = 1:3) mock_response$request <- list( policies = list( @@ -35,7 +30,7 @@ test_that("resp_tidy parses httr2_response objects with resp_tidy policy (#40)", expect_identical(test_result, 1:3) }) -test_that("resp_tidy uses policies$resp_tidy$tidy_args (#40)", { +test_that("resp_tidy uses policies$resp_tidy$tidy_args (#40, #88)", { mock_response <- httr2::response_json(body = 1:3) mock_response$request <- list( policies = list( @@ -50,44 +45,3 @@ test_that("resp_tidy uses policies$resp_tidy$tidy_args (#40)", { test_result <- resp_tidy(mock_response) expect_identical(test_result, 1:6) }) - -test_that("resp_tidy parses and combines nectar_responses objects (#40)", { - request_obj <- list( - policies = list( - resp_tidy = list( - tidy_fn = function(resp) { - unlist(httr2::resp_body_json(resp)) - } - ) - ) - ) - mock_response1 <- httr2::response_json(body = 1:3) - mock_response1$request <- request_obj - mock_response2 <- httr2::response_json(body = 4:6) - mock_response2$request <- request_obj - mock_responses <- structure( - list(mock_response1, mock_response2), - class = c("nectar_responses", "list") - ) - test_result <- resp_tidy(mock_responses) - expect_identical(test_result, 1:6) -}) - -test_that("resp_tidy parses and combines lists of httr2_response objects (#40)", { - request_obj <- list( - policies = list( - resp_tidy = list( - tidy_fn = function(resp) { - unlist(httr2::resp_body_json(resp)) - } - ) - ) - ) - mock_response1 <- httr2::response_json(body = 1:3) - mock_response1$request <- request_obj - mock_response2 <- httr2::response_json(body = 4:6) - mock_response2$request <- request_obj - mock_responses <- list(mock_response1, mock_response2) - test_result <- resp_tidy(mock_responses) - expect_identical(test_result, 1:6) -}) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 0bec1d5..8e2439b 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -1,4 +1,39 @@ -test_that("Can build clean urls", { +# .check_httr2_response -------------------------------------------------------- + +test_that(".check_httr2_response returns input for valid httr2_response (#noissue)", { + mock_response <- httr2::response_json(body = list(a = 1)) + expect_identical(.check_httr2_response(mock_response), mock_response) +}) + +test_that(".check_httr2_response errors with not_httr2_response for non-response (#noissue)", { + expect_nectar_error_snapshot( + .check_httr2_response(1), + "not_httr2_response" + ) +}) + +# compact_nested_list ---------------------------------------------------------- + +test_that("compact_nested_list removes NULLs recursively (#noissue)", { + x <- list( + a = list(b = 1, c = NULL), + d = NULL, + e = 2 + ) + expect_identical( + compact_nested_list(x), + list(a = list(b = 1), e = 2) + ) +}) + +test_that(".compact_nested_list_impl skips recursion at depth 20 (#noissue)", { + result <- .compact_nested_list_impl(list(a = 1, b = NULL), depth = 20L) + expect_identical(result, list(a = 1)) +}) + +# url_path_append / url_normalize ---------------------------------------------- + +test_that("Can build clean urls (#noissue)", { expected_result <- "https://example.com/api/v1/users" expect_identical( url_path_append("https://example.com", "api", "v1", "users"), @@ -13,3 +48,69 @@ test_that("Can build clean urls", { expected_result ) }) + +test_that("url_normalize produces the same URL with or without a trailing slash (#noissue)", { + expect_identical( + url_normalize("https://example.com"), + url_normalize("https://example.com/") + ) +}) + +# do_if_fn_defined ------------------------------------------------------------- + +test_that("do_if_fn_defined returns x unchanged when fn is NULL (#noissue)", { + expect_identical(do_if_fn_defined(42L), 42L) +}) + +test_that("do_if_fn_defined applies fn to x when fn is provided (#noissue)", { + expect_identical(do_if_fn_defined(5, \(x) x * 2), 10) + expect_identical(do_if_fn_defined(5, \(x, y) x + y, 3), 8) +}) + +# .do_if_args_defined ---------------------------------------------------------- + +test_that(".do_if_args_defined returns x unchanged when all args are NULL (#noissue)", { + expect_identical(.do_if_args_defined(42L, sum, a = NULL), 42L) +}) + +test_that(".do_if_args_defined applies fn when non-NULL args are provided (#noissue)", { + expect_identical(.do_if_args_defined(5, \(x, y) x + y, y = 3), 8) +}) + +# get_pkg_name ----------------------------------------------------------------- + +test_that("get_pkg_name returns NULL when called outside a package (#noissue)", { + expect_null(get_pkg_name(globalenv())) +}) + +test_that("get_pkg_name returns the package name when called from a package namespace (#noissue)", { + expect_identical( + get_pkg_name(rlang::ns_env("rlang")), + "rlang" + ) +}) + +# .get_pkg_version ------------------------------------------------------------- + +test_that(".get_pkg_version returns a character version for an installed package (#noissue)", { + expect_identical( + .get_pkg_version("base"), + paste(R.Version()$major, R.Version()$minor, sep = ".") + ) +}) + +test_that(".get_pkg_version errors for a non-string pkg_name (#noissue)", { + stbl::expect_pkg_error_classes( + .get_pkg_version(mean), + "stbl", + "coerce", + "character" + ) +}) + +test_that(".get_pkg_version errors for an uninstalled package (#noissue)", { + expect_error( + .get_pkg_version("nonexistent_package_xyz_abc"), + "required to find the package version" + ) +})