More clearly differentiate resp_parse() vs resp_tidy()

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)

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)
 }

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)
 }

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

tests/testthat/_snaps/resp_parse.md

@@ -6,6 +6,17 @@
     Output
       <error/nectar-error-unsupported_response_class>
       Error:
-      ! `1` must be a <list> or a <httr2_response>.
+      ! `1` must be a <httr2_response> or a <list> of <httr2_response> 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/nectar-error-unsupported_response_class>
+      Error in `resp_parse()`:
+      ! `resps` must be a list of <httr2_response> objects.
+      x Not all elements of `resps` are <httr2_response> objects.
+

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/nectar-error-unsupported_response_class>
+      <error/nectar-error-not_httr2_response>
       Error in `resp_tidy()`:
-      ! No method is available to `nectar::resp_tidy()` this object.
-      i `nectar::resp_tidy()` expects <httr2_response> 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/nectar-error-unsupported_response_class>
-      Error in `resp_tidy()`:
-      ! No method is available to `nectar::resp_tidy()` this object.
-      i `nectar::resp_tidy()` expects <httr2_response> objects, or lists thereof.
+      ! `x` must be a <httr2_response> object.
+      x `x` is a number.
 

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/nectar-error-not_httr2_response>
+      Error:
+      ! `x` must be a <httr2_response> object.
+      x `x` is a number.
+

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")