Overhaul modify_list() (#130)

Now uses strategy suggested by @jchrom, where we operate on "homonym groups". This makes it possible to use duplicate names in url parameters and form/multipart bodies.

Fixes #97. Fixes #107.

NEWS.md

@@ -1,5 +1,8 @@
 # httr2 (development version)
 
+* `req_body_form()`, `req_body_multipart()`, and `req_url_query()` now 
+  support multiple arguments with the same name (#97, #107).
+
 * `req_throttle()` correctly sets throttle rate (@jchrom, #101).
 
 * `req_error()` can now correct force successful HTTP statuses to fail (#98).

R/utils.R

@@ -28,8 +28,10 @@ modify_list <- function(.x, ...) {
   if (!is_named(dots)) {
     abort("All components of ... must be named")
   }
-  .x[names(dots)] <- dots
-  out <- compact(.x)
+
+  out <- .x[!names(.x) %in% names(dots)]
+  out <- c(out, compact(dots))
+
   if (length(out) == 0) {
     names(out) <- NULL
   }

tests/testthat/test-req-body.R

@@ -49,10 +49,13 @@ test_that("can send named list as json/form/multipart", {
 
 test_that("can modify body data", {
   req1 <- request_test() %>% req_body_form(list(a = 1))
-  req2 <- req1 %>% req_body_form(list(b = 2))
-
   expect_equal(req1$body$data, list(a = 1))
+
+  req2 <- req1 %>% req_body_form(list(b = 2))
   expect_equal(req2$body$data, list(a = 1, b = 2))
+
+  req3 <- req1 %>% req_body_form(list(a = 3, a = 4))
+  expect_equal(req3$body$data, list(a = 3, a = 4))
 })
 
 test_that("can upload file with multipart", {

tests/testthat/test-req-url.R

@@ -30,6 +30,9 @@ test_that("can set query params", {
   expect_equal(req_url_query(req, a = 1, b = 2)$url, "http://example.com/?a=1&b=2")
   expect_equal(req_url_query(req, a = 1, b = 2, c = NULL)$url, "http://example.com/?a=1&b=2")
   expect_equal(req_url_query(req, !!!list(a = 1, b = 2))$url, "http://example.com/?a=1&b=2")
+
+  expect_equal(req_url_query(req, a = 1, a = 2)$url, "http://example.com/?a=1&a=2")
+  expect_equal(req_url_query(req, !!!list(a = 1, a = 2))$url, "http://example.com/?a=1&a=2")
 })
 
 test_that("empty query doesn't affect url", {
@@ -41,7 +44,8 @@ test_that("empty query doesn't affect url", {
 test_that("can modify query params iteratively", {
   req <- request("http://example.com/?a=1&b=2")
   expect_equal(req_url_query(req, c = 3)$url, "http://example.com/?a=1&b=2&c=3")
-  expect_equal(req_url_query(req, a = 2)$url, "http://example.com/?a=2&b=2")
+  expect_equal(req_url_query(req, a = 2)$url, "http://example.com/?b=2&a=2")
+  expect_equal(req_url_query(req, a = 1, a = 2)$url, "http://example.com/?b=2&a=1&a=2")
   expect_equal(req_url_query(req, b = NULL)$url, "http://example.com/?a=1")
 })
 

tests/testthat/test-utils.R

@@ -9,6 +9,13 @@ test_that("modify list adds, removes, and overrides", {
   expect_snapshot(modify_list(x, a = 1, 2), error = TRUE)
 })
 
+test_that("replacement affects all components with name", {
+  x <- list(a = 1, a = 2)
+  expect_equal(modify_list(x, a = NULL), list())
+  expect_equal(modify_list(x, a = 3), list(a = 3))
+  expect_equal(modify_list(x, a = 3, a = 4), list(a = 3, a =4))
+})
+
 test_that("can check arg types", {
   expect_snapshot(error = TRUE, {
     check_string(1, "x")