Export req_perform_opinionated (#24)

NAMESPACE

@@ -7,6 +7,7 @@ S3method(resp_parse,httr2_response)
 S3method(resp_parse,list)
 export(call_api)
 export(compact_nested_list)
+export(req_perform_opinionated)
 export(req_prepare)
 export(resp_parse)
 export(security_api_key)

R/call.R

@@ -7,14 +7,14 @@
 #' [httr2::resp_body_json()].
 #'
 #' @inheritParams .shared-parameters
-#' @inheritParams httr2::req_perform_iterative
+#' @inheritParams req_perform_opinionated
 #' @param response_parser_args An optional list of arguments to pass to the
 #'   `response_parser` function (in addition to `resp`).
 #' @param next_req An optional function that takes the previous response
 #'   (`resp`) to generate the next request in a call to
 #'   [httr2::req_perform_iterative()]. This function can usually be generated
-#'   using [httr2::iterate_with_offset()], [httr2::iterate_with_cursor()], or
-#'   [httr2::iterate_with_link_url()].
+#'   using one of the iteration helpers described in
+#'   [httr2::iterate_with_offset()].
 #'
 #' @return The response from the API, parsed by the `response_parser`.
 #' @export
@@ -30,6 +30,7 @@ call_api <- function(base_url,
                      response_parser_args = list(),
                      next_req = NULL,
                      max_reqs = Inf,
+                     max_tries_per_req = 3,
                      user_agent = "nectar (https://nectar.api2r.org)") {
   req <- req_prepare(
     base_url = base_url,
@@ -41,7 +42,12 @@ call_api <- function(base_url,
     user_agent = user_agent
   )
   req <- .req_security_apply(req, security_fn, security_args)
-  resp <- .req_perform(req, next_req, max_reqs)
+  resp <- req_perform_opinionated(
+    req,
+    next_req = next_req,
+    max_reqs = max_reqs,
+    max_tries_per_req = max_tries_per_req
+  )
   resp <- resp_parse(
     resp,
     response_parser = response_parser,
@@ -58,21 +64,3 @@ call_api <- function(base_url,
   }
   return(req)
 }
-
-.req_perform <- function(req, next_req = NULL, max_reqs = Inf) {
-  if (is.null(next_req)) {
-    return(req_perform(req))
-  }
-  req <- httr2::req_retry(
-    req,
-    max_tries = 4,
-    backoff = function(attempts) {
-      10 * attempts # nocov
-    }
-  )
-  req_perform_iterative(
-    req,
-    next_req = next_req,
-    max_reqs = max_reqs
-  )
-}

R/perform.R

@@ -0,0 +1,46 @@
+#' Perform a request with opinionated defaults
+#'
+#' This function ensures that a request has [httr2::req_retry()] applied, and
+#' then performs the request, using either [httr2::req_perform_iterative()] (if
+#' a `next_req` function is supplied) or [httr2::req_perform()] (if not).
+#'
+#' @inheritParams .shared-parameters
+#' @inheritParams httr2::req_perform_iterative
+#' @inheritParams rlang::args_dots_empty
+#' @param max_reqs The maximum number of separate requests to perform. Passed to
+#'   the max_reqs argument of [httr2::req_perform_iterative()] when `next_req`
+#'   is supplied. The default `2` should likely be changed to `Inf` after you
+#'   validate the function.
+#' @param max_tries_per_req The maximum number of times to attempt each
+#'   individual request. Passed to the `max_tries` argument of
+#'   [httr2::req_retry()].
+#'
+#' @return A list of [httr2::response()] objects, one for each request
+#'   performed.
+#' @export
+req_perform_opinionated <- function(req,
+                                    ...,
+                                    next_req = NULL,
+                                    max_reqs = 2,
+                                    max_tries_per_req = 3) {
+  rlang::check_dots_empty()
+  req <- .req_apply_retry_default(req, max_tries_per_req)
+  if (is.null(next_req)) {
+    return(list(req_perform(req)))
+  }
+  req_perform_iterative(
+    req,
+    next_req = next_req,
+    max_reqs = max_reqs
+  )
+}
+
+.req_apply_retry_default <- function(req, max_tries_per_req) {
+  if (
+    any(c("retry_max_wait", "retry_max_tries") %in% names(req$policies)) ||
+      is.null(max_tries_per_req)
+  ) {
+    return(req)
+  }
+  return(httr2::req_retry(req, max_tries = max_tries_per_req))
+}

R/resp.R

@@ -49,9 +49,11 @@ resp_parse.httr2_response <- function(resp,
                                       ...,
                                       response_parser = httr2::resp_body_json) {
   if (length(response_parser)) {
-    return(.resp_parse_apply(resp, response_parser, ...))
+    # Higher-level calls can include !!!'ed arguments.
+    dots <- rlang::list2(...)
+    return(rlang::inject(response_parser(resp, !!!dots)))
   }
-  resp
+  return(resp)
 }
 
 #' @export
@@ -65,10 +67,3 @@ resp_parse.list <- function(resp,
     }
   )
 }
-
-
-.resp_parse_apply <- function(resp, response_parser, ...) {
-  # Higher-level calls can include !!!'ed arguments.
-  dots <- rlang::list2(...)
-  return(rlang::inject(response_parser(resp, !!!dots)))
-}

tests/testthat/test-call.R

@@ -1,36 +1,31 @@
 test_that("call_api() calls an API", {
   local_mocked_bindings(
-    req_perform = function(req) {
-      structure(req, class = "httr2_response")
+    req_perform_opinionated = function(req, ...) list("performed"),
+    resp_parse = function(resp, ...) {
+      identical(resp, list("performed"))
     }
   )
   expect_no_error({
     test_result <- call_api(
       base_url = "https://example.com",
-      response_parser = NULL,
       user_agent = NULL
     )
   })
-  expect_identical(
-    test_result$url,
-    "https://example.com/"
-  )
+  expect_true(test_result)
 })
 
 test_that("call_api() applies security", {
   local_mocked_bindings(
-    req_perform = function(req) {
-      structure(req, class = "httr2_response")
-    }
+    req_perform_opinionated = function(req, ...) req,
+    resp_parse = function(resp, ...) resp
   )
   test_result <- call_api(
     base_url = "https://example.com",
     user_agent = NULL,
     security_fn = httr2::req_url_query,
     security_args = list(
       security = "set"
-    ),
-    response_parser = NULL
+    )
   )
   expect_identical(
     test_result$url,
@@ -40,8 +35,8 @@ test_that("call_api() applies security", {
 
 test_that("call_api() uses response_parser", {
   local_mocked_bindings(
-    req_perform = function(req) {
-      httr2::response(body = "specific text")
+    req_perform_opinionated = function(req, ...) {
+      list(httr2::response(body = "specific text"))
     }
   )
   parser <- function(resp) {
@@ -54,20 +49,3 @@ test_that("call_api() uses response_parser", {
   )
   expect_true(test_result)
 })
-
-test_that("call_api() applies iteration when appropriate", {
-  local_mocked_bindings(
-    req_perform = function(req) {
-      "req_perform"
-    },
-    req_perform_iterative = function(req, next_req = NULL, max_reqs = Inf) {
-      "req_perform_iterative"
-    }
-  )
-  req <- httr2::request("https://example.com")
-  expect_identical(.req_perform(req), "req_perform")
-  expect_identical(
-    .req_perform(req, next_req = mean),
-    "req_perform_iterative"
-  )
-})

tests/testthat/test-perform.R

@@ -0,0 +1,33 @@
+test_that("req_perform_opinionated() applies iteration when appropriate", {
+  local_mocked_bindings(
+    req_perform = function(req) {
+      "req_perform"
+    },
+    req_perform_iterative = function(req, next_req = NULL, max_reqs = Inf) {
+      "req_perform_iterative"
+    }
+  )
+  req <- httr2::request("https://example.com")
+  expect_identical(req_perform_opinionated(req), list("req_perform"))
+  expect_identical(
+    req_perform_opinionated(req, next_req = c),
+    "req_perform_iterative"
+  )
+})
+
+test_that("req_perform_opinionated applies retries when appropriate", {
+  local_mocked_bindings(
+    req_perform = function(req) req,
+    req_perform_iterative = function(req, ...) req
+  )
+  req <- httr2::request("https://example.com")
+  expect_identical(
+    req_perform_opinionated(req),
+    list(httr2::req_retry(req, max_tries = 3))
+  )
+  req_with_retries <- httr2::req_retry(req, max_seconds = 10)
+  expect_identical(
+    req_perform_opinionated(req_with_retries),
+    list(req_with_retries)
+  )
+})