From ca6cf4e799bdadd189d69e7e29023181d05c226c Mon Sep 17 00:00:00 2001 From: Maximilian Girlich Date: Wed, 2 Aug 2023 13:24:46 +0000 Subject: [PATCH 1/3] Parse query components in `curl_translate()` --- NEWS.md | 2 ++ R/curl.R | 35 +++++++++++++++++++++++++++-------- tests/testthat/_snaps/curl.md | 12 ++++++++++++ tests/testthat/test-curl.R | 6 ++++++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/NEWS.md b/NEWS.md index f6f676fc..965e2625 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # httr2 (development version) +* `curl_translate()` now parses the query components of the url (@mgirlich, #259). + * New `resp_has_body()` returns a `TRUE` or `FALSE` depending on whether or not the response has a body (#205). diff --git a/R/curl.R b/R/curl.R index 9ecc92df..d5e79003 100644 --- a/R/curl.R +++ b/R/curl.R @@ -37,8 +37,17 @@ curl_translate <- function(cmd) { } data <- curl_normalize(cmd) - out <- glue('request("{data$url}")') + url_pieces <- httr2::url_parse(data$url) + query <- url_pieces$query + url_pieces$query <- NULL + url <- url_build(url_pieces) + + out <- glue('request("{url}")') add_line <- function(x, y) { + if (is.null(y)) { + return(x) + } + paste0(x, ' %>% \n ', gsub("\n", "\n ", y)) } @@ -46,17 +55,15 @@ curl_translate <- function(cmd) { out <- add_line(out, glue('req_method("{data$method}")')) } + step_query <- curl_step("req_url_query", query) + out <- add_line(out, step_query) + # Content type set with data type <- data$headers$`Content-Type` data$headers$`Content-Type` <- NULL - if (length(data$headers) > 0) { - names <- quote_name(names(data$headers)) - values <- encodeString(unlist(data$headers), quote = '"') - args <- paste0(" ", names, " = ", values, ",\n", collapse = "") - - out <- add_line(out, paste0("req_headers(\n", args, ")")) - } + step_headers <- curl_step("req_headers", data$headers) + out <- add_line(out, step_headers) if (!identical(data$data, "")) { type <- encodeString(type %||% "application/x-www-form-urlencoded", quote = '"') @@ -234,3 +241,15 @@ is_syntactic <- function(x) { quote_name <- function(x) { ifelse(is_syntactic(x), x, encodeString(x, quote = "`")) } + +curl_step <- function(f, args) { + if (is_empty(args)) { + return() + } + + names <- quote_name(names(args)) + values <- encodeString(unlist(args), quote = '"') + args_string <- paste0(" ", names, " = ", values, ",\n", collapse = "") + + paste0(f, "(\n", args_string, ")") +} diff --git a/tests/testthat/_snaps/curl.md b/tests/testthat/_snaps/curl.md index 95d2f6ec..9e9398c8 100644 --- a/tests/testthat/_snaps/curl.md +++ b/tests/testthat/_snaps/curl.md @@ -47,6 +47,18 @@ request("http://x.com") %>% req_perform(verbosity = 1) +# can translate query + + Code + curl_translate("curl http://x.com?string=abcde&b=2") + Output + request("http://x.com") %>% + req_url_query( + string = "abcde", + b = "2", + ) %>% + req_perform() + # can translate data Code diff --git a/tests/testthat/test-curl.R b/tests/testthat/test-curl.R index a1294b97..edd26386 100644 --- a/tests/testthat/test-curl.R +++ b/tests/testthat/test-curl.R @@ -96,6 +96,12 @@ test_that("can translate to httr calls", { }) }) +test_that("can translate query", { + expect_snapshot({ + curl_translate("curl http://x.com?string=abcde&b=2") + }) +}) + test_that("can translate data", { expect_snapshot({ curl_translate("curl http://example.com --data abcdef") From 2b9ebae78374d961faffe8cd0539b5610052dcf3 Mon Sep 17 00:00:00 2001 From: Maximilian Girlich Date: Wed, 2 Aug 2023 13:49:32 +0000 Subject: [PATCH 2/3] Refactor step logic in `curl_translate()` --- R/curl.R | 59 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/R/curl.R b/R/curl.R index d5e79003..5e5f5e6e 100644 --- a/R/curl.R +++ b/R/curl.R @@ -42,44 +42,31 @@ curl_translate <- function(cmd) { url_pieces$query <- NULL url <- url_build(url_pieces) - out <- glue('request("{url}")') - add_line <- function(x, y) { - if (is.null(y)) { - return(x) - } - - paste0(x, ' %>% \n ', gsub("\n", "\n ", y)) - } - - if (!is.null(data$method)) { - out <- add_line(out, glue('req_method("{data$method}")')) - } + steps <- glue('request("{url}")') + steps <- add_curl_step(steps, "req_method", data$method, trailing_comma = FALSE) - step_query <- curl_step("req_url_query", query) - out <- add_line(out, step_query) + steps <- add_curl_step(steps, "req_url_query", query, trailing_comma = TRUE) # Content type set with data type <- data$headers$`Content-Type` data$headers$`Content-Type` <- NULL - step_headers <- curl_step("req_headers", data$headers) - out <- add_line(out, step_headers) + steps <- add_curl_step(steps, "req_headers", data$headers, trailing_comma = TRUE) if (!identical(data$data, "")) { - type <- encodeString(type %||% "application/x-www-form-urlencoded", quote = '"') - body <- encodeString(data$data, quote = '"') - out <- add_line(out, glue("req_body_raw({body}, {type})")) + type <- type %||% "application/x-www-form-urlencoded" + body <- data$data + steps <- add_curl_step(steps, "req_body_raw", c(body, type), trailing_comma = FALSE) } - if (!is.null(data$auth)) { - out <- add_line(out, glue('req_auth_basic("{data$auth[[1]]}", "{data$auth[[2]]}")')) - } + steps <- add_curl_step(steps, "req_auth_basic", unname(data$auth), trailing_comma = FALSE) if (data$verbose) { - out <- add_line(out, "req_perform(verbosity = 1)") + steps <- c(steps, "req_perform(verbosity = 1)") } else { - out <- add_line(out, "req_perform()") + steps <- c(steps, "req_perform()") } + out <- paste0(steps, collapse = " %>% \n ") out <- paste0(out, "\n") if (clip) { @@ -236,20 +223,32 @@ curl_args <- function(cmd) { # Helpers ----------------------------------------------------------------- is_syntactic <- function(x) { - x == make.names(x) + x == "" | x == make.names(x) } quote_name <- function(x) { ifelse(is_syntactic(x), x, encodeString(x, quote = "`")) } -curl_step <- function(f, args) { +add_curl_step <- function(steps, f, args, trailing_comma) { if (is_empty(args)) { - return() + return(steps) } - names <- quote_name(names(args)) + names <- quote_name(names2(args)) values <- encodeString(unlist(args), quote = '"') - args_string <- paste0(" ", names, " = ", values, ",\n", collapse = "") - paste0(f, "(\n", args_string, ")") + args_named <- ifelse( + names == "", + paste0(values), + paste0(names, " = ", values) + ) + if (trailing_comma) { + args_string <- paste0(" ", args_named, ",\n", collapse = "") + new_step <- paste0(f, "(\n", args_string, " )") + } else { + args_string <- paste0(args_named, collapse = ", ") + new_step <- paste0(f, "(", args_string, ")") + } + + c(steps, new_step) } From e567ceefa4f64d637617873ce98977cb26845ff5 Mon Sep 17 00:00:00 2001 From: Maximilian Girlich Date: Wed, 2 Aug 2023 14:19:57 +0000 Subject: [PATCH 3/3] Refactor `add_curl_steps()` --- R/curl.R | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/R/curl.R b/R/curl.R index 5e5f5e6e..193e280e 100644 --- a/R/curl.R +++ b/R/curl.R @@ -43,29 +43,30 @@ curl_translate <- function(cmd) { url <- url_build(url_pieces) steps <- glue('request("{url}")') - steps <- add_curl_step(steps, "req_method", data$method, trailing_comma = FALSE) + steps <- add_curl_step(steps, "req_method", main_args = data$method) - steps <- add_curl_step(steps, "req_url_query", query, trailing_comma = TRUE) + steps <- add_curl_step(steps, "req_url_query", dots = query) # Content type set with data type <- data$headers$`Content-Type` data$headers$`Content-Type` <- NULL - steps <- add_curl_step(steps, "req_headers", data$headers, trailing_comma = TRUE) + steps <- add_curl_step(steps, "req_headers", dots = data$headers) if (!identical(data$data, "")) { type <- type %||% "application/x-www-form-urlencoded" body <- data$data - steps <- add_curl_step(steps, "req_body_raw", c(body, type), trailing_comma = FALSE) + steps <- add_curl_step(steps, "req_body_raw", main_args = c(body, type)) } - steps <- add_curl_step(steps, "req_auth_basic", unname(data$auth), trailing_comma = FALSE) + steps <- add_curl_step(steps, "req_auth_basic", main_args = unname(data$auth)) + perform_args <- list() if (data$verbose) { - steps <- c(steps, "req_perform(verbosity = 1)") - } else { - steps <- c(steps, "req_perform()") + perform_args$verbosity <- 1 } + steps <- add_curl_step(steps, "req_perform", main_args = perform_args, keep_if_empty = TRUE) + out <- paste0(steps, collapse = " %>% \n ") out <- paste0(out, "\n") @@ -229,25 +230,35 @@ quote_name <- function(x) { ifelse(is_syntactic(x), x, encodeString(x, quote = "`")) } -add_curl_step <- function(steps, f, args, trailing_comma) { - if (is_empty(args)) { +add_curl_step <- function(steps, + f, + ..., + main_args = NULL, + dots = NULL, + keep_if_empty = FALSE) { + check_dots_empty0(...) + args <- c(main_args, dots) + + if (is_empty(args) && !keep_if_empty) { return(steps) } names <- quote_name(names2(args)) - values <- encodeString(unlist(args), quote = '"') + string <- vapply(args, is.character, logical(1L)) + values <- unlist(args) + values <- ifelse(string, encodeString(values, quote = '"'), values) args_named <- ifelse( names == "", paste0(values), paste0(names, " = ", values) ) - if (trailing_comma) { - args_string <- paste0(" ", args_named, ",\n", collapse = "") - new_step <- paste0(f, "(\n", args_string, " )") - } else { + if (is_empty(dots)) { args_string <- paste0(args_named, collapse = ", ") new_step <- paste0(f, "(", args_string, ")") + } else { + args_string <- paste0(" ", args_named, ",\n", collapse = "") + new_step <- paste0(f, "(\n", args_string, " )") } c(steps, new_step)