Skip to content

Commit

Permalink
Check the result of parse_resp(resp)
Browse files Browse the repository at this point in the history
  • Loading branch information
mgirlich committed Sep 22, 2023
1 parent bf12393 commit bd9cd59
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 2 deletions.
24 changes: 23 additions & 1 deletion R/paginate.R
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#' * `next_url` for `paginate_next_url()`.
#' * `next_token` for `paginate_next_token()`.
#'
#' @param required_fields An optional character vector that describes which
#' fields are required in the list returned by `parse_resp()`.
#' @param n_pages An optional function that extracts the total number of pages, improving the
#' automatically generated progress bar. It has two arguments:
#'
Expand Down Expand Up @@ -63,19 +65,37 @@
req_paginate <- function(req,
next_request,
parse_resp = NULL,
required_fields = NULL,
n_pages = NULL) {
check_request(req)
check_function2(next_request, args = c("req", "parsed"))
check_function2(parse_resp, args = "resp", allow_null = TRUE)
parse_resp <- parse_resp %||% function(resp) list(data = resp)
check_character(required_fields, allow_null = TRUE)
required_fields <- union(required_fields, "data")
check_function2(n_pages, args = "parsed", allow_null = TRUE)
n_pages <- n_pages %||% function(parsed) Inf

wrapped_parse_resp <- function(resp) {
out <- parse_resp(resp)
if (!is.list(out)) {
cli::cli_abort("{.fun parse_resp} must return a list, not {.obj_type_friendly {out}}.")
}

missing_fields <- setdiff(required_fields, names2(out))
if (!is_empty(missing_fields)) {
cli::cli_abort(c("The list returned by {.code parse_resp(resp)} is missing the field{?s} {.field {missing_fields}}."))
}

out
}

req_policies(
req,
paginate = list(
next_request = next_request,
parse_resp = parse_resp,
parse_resp = wrapped_parse_resp,
required_fields = required_fields,
n_pages = n_pages
)
)
Expand Down Expand Up @@ -205,6 +225,7 @@ req_paginate_next_url <- function(req,
req,
next_request,
parse_resp = parse_resp,
required_fields = "next_url",
n_pages = n_pages
)
}
Expand Down Expand Up @@ -237,6 +258,7 @@ req_paginate_token <- function(req,
req,
next_request,
parse_resp = parse_resp,
required_fields = "next_token",
n_pages = n_pages
)
}
Expand Down
26 changes: 26 additions & 0 deletions tests/testthat/_snaps/paginate.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,32 @@
Error in `paginate_req_perform()`:
! `progress` must be a bool, a string, or a list, not the number -1.

# parse_resp() produces a good error message

Code
req_not_a_list$policies$paginate$parse_resp(resp)
Condition
Error in `req_not_a_list$policies$paginate$parse_resp()`:
! `parse_resp()` must return a list, not a string.
Code
req_missing_1_field$policies$paginate$parse_resp(resp)
Condition
Error in `req_missing_1_field$policies$paginate$parse_resp()`:
! The list returned by `parse_resp(resp)` is missing the field next_url.
Code
req_missing_2_field$policies$paginate$parse_resp(resp)
Condition
Error in `req_missing_2_field$policies$paginate$parse_resp()`:
! The list returned by `parse_resp(resp)` is missing the fields next_url and data.

---

Code
paginate_req_perform(req, max_pages = 2)
Condition
Error in `parse_resp()`:
! The list returned by `parse_resp(resp)` is missing the field next_token.

# paginate_req_perform() handles error in `parse_resp()`

Code
Expand Down
31 changes: 30 additions & 1 deletion tests/testthat/test-paginate.R
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ test_that("req_paginate_next_url() can paginate", {
req1 <- request("https://pokeapi.co/api/v2/pokemon") %>%
req_url_query(limit = 11) %>%
req_paginate_next_url(
parse_resp = function(resp) list(next_url = resp_body_json(resp)[["next"]])
parse_resp = function(resp) list(data = "a", next_url = resp_body_json(resp)[["next"]])
)

resp <- req_perform(req1)
Expand Down Expand Up @@ -208,6 +208,35 @@ test_that("paginate_req_perform() works if there is only one page", {
expect_length(responses, 1)
})

test_that("parse_resp() produces a good error message", {
req_not_a_list <- request("https://pokeapi.co/api/v2/pokemon") %>%
req_paginate_next_url(parse_resp = function(resp) "a")
req_missing_1_field <- request("https://pokeapi.co/api/v2/pokemon") %>%
req_paginate_next_url(parse_resp = function(resp) list(data = "a"))
req_missing_2_field <- request("https://pokeapi.co/api/v2/pokemon") %>%
req_paginate_next_url(parse_resp = function(resp) list(x = "a"))
resp <- response()

expect_snapshot(error = TRUE, {
req_not_a_list$policies$paginate$parse_resp(resp)
req_missing_1_field$policies$paginate$parse_resp(resp)
req_missing_2_field$policies$paginate$parse_resp(resp)
})

# The error call is helpful
req <- request_pagination_test(
parse_resp = function(resp) {
parsed <- resp_body_json(resp)

list(parsed$my_next_token, data = parsed)
}
)

expect_snapshot(error = TRUE, {
paginate_req_perform(req, max_pages = 2)
})
})

test_that("paginate_req_perform() handles error in `parse_resp()`", {
req <- request_pagination_test(
parse_resp = function(resp) {
Expand Down

0 comments on commit bd9cd59

Please sign in to comment.