From cf92bd8f0c6f1ecdcc44b3a55e05146af3efa84f Mon Sep 17 00:00:00 2001 From: Jon Harmon Date: Fri, 26 Apr 2024 09:38:43 -0500 Subject: [PATCH] Establish and apply formal principles (#28) --- .Rbuildignore | 1 + NAMESPACE | 4 +- R/{aaa_shared.R => aaa-shared.R} | 0 R/{security_api_key.R => auth.R} | 29 ++++--- R/call.R | 4 +- R/req.R | 25 +----- R/req_body.R | 5 +- R/req_method.R | 14 ++++ R/req_path.R | 19 +++++ R/req_query.R | 20 +++-- R/resp.R | 2 +- R/utils.R | 31 +++++++- man/call_api.Rd | 14 ++-- man/{do_if_defined.Rd => do_if_fn_defined.Rd} | 8 +- man/dot-do_if_args_defined.Rd | 24 ++++++ ...okie.Rd => dot-req_auth_api_key_cookie.Rd} | 10 +-- ...ader.Rd => dot-req_auth_api_key_header.Rd} | 8 +- man/dot-req_auth_api_key_query.Rd | 22 ++++++ man/dot-req_method_apply.Rd | 22 ++++++ man/dot-req_path_append.Rd | 23 ++++++ man/dot-req_query_flatten.Rd | 23 ++++++ man/dot-shared-parameters.Rd | 2 +- man/dot-shared-request.Rd | 2 +- ...ecurity_api_key.Rd => req_auth_api_key.Rd} | 8 +- man/req_modify.Rd | 12 ++- man/req_prepare.Rd | 12 ++- principles.md | 26 ++++++ tests/testthat/test-auth.R | 79 +++++++++++++++++++ tests/testthat/test-req_query.R | 5 +- tests/testthat/test-security_api_key.R | 43 ---------- 30 files changed, 370 insertions(+), 127 deletions(-) rename R/{aaa_shared.R => aaa-shared.R} (100%) rename R/{security_api_key.R => auth.R} (67%) create mode 100644 R/req_method.R create mode 100644 R/req_path.R rename man/{do_if_defined.Rd => do_if_fn_defined.Rd} (90%) create mode 100644 man/dot-do_if_args_defined.Rd rename man/{dot-security_api_key_cookie.Rd => dot-req_auth_api_key_cookie.Rd} (62%) rename man/{dot-security_api_key_header.Rd => dot-req_auth_api_key_header.Rd} (72%) create mode 100644 man/dot-req_auth_api_key_query.Rd create mode 100644 man/dot-req_method_apply.Rd create mode 100644 man/dot-req_path_append.Rd create mode 100644 man/dot-req_query_flatten.Rd rename man/{security_api_key.Rd => req_auth_api_key.Rd} (85%) create mode 100644 principles.md create mode 100644 tests/testthat/test-auth.R delete mode 100644 tests/testthat/test-security_api_key.R diff --git a/.Rbuildignore b/.Rbuildignore index 87ad3b7..d7823b8 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -8,3 +8,4 @@ ^_pkgdown\.yml$ ^docs$ ^pkgdown$ +^principles\.md$ diff --git a/NAMESPACE b/NAMESPACE index ecd0ab4..91cbea0 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -7,13 +7,13 @@ S3method(resp_parse,httr2_response) S3method(resp_parse,list) export(call_api) export(compact_nested_list) -export(do_if_defined) +export(do_if_fn_defined) +export(req_auth_api_key) export(req_modify) export(req_perform_opinionated) export(req_prepare) export(req_setup) export(resp_parse) -export(security_api_key) export(stabilize_string) export(url_normalize) export(url_path_append) diff --git a/R/aaa_shared.R b/R/aaa-shared.R similarity index 100% rename from R/aaa_shared.R rename to R/aaa-shared.R diff --git a/R/security_api_key.R b/R/auth.R similarity index 67% rename from R/security_api_key.R rename to R/auth.R index 4de59d8..c3dcceb 100644 --- a/R/security_api_key.R +++ b/R/auth.R @@ -15,13 +15,13 @@ #' #' @inherit .shared-request return #' @export -security_api_key <- function(req, +req_auth_api_key <- function(req, ..., location = "header") { switch(location, - header = .security_api_key_header(req, ...), - query = .security_api_key_query(req, ...), - cookie = .security_api_key_cookie(req, ...) # nocov + header = .req_auth_api_key_header(req, ...), + query = .req_auth_api_key_query(req, ...), + cookie = .req_auth_api_key_cookie(req, ...) # nocov ) } @@ -33,7 +33,7 @@ security_api_key <- function(req, #' #' @inherit .shared-request return #' @keywords internal -.security_api_key_header <- function(req, ..., parameter_name, api_key) { +.req_auth_api_key_header <- function(req, ..., parameter_name, api_key = NULL) { rlang::check_dots_empty() if (length(api_key) && nchar(api_key)) { req <- httr2::req_headers( @@ -45,7 +45,15 @@ security_api_key <- function(req, return(req) } -.security_api_key_query <- function(req, ..., parameter_name, api_key) { +#' Authenticate with an API key in the query of the request +#' +#' @inheritParams .shared-parameters +#' @param parameter_name The name to use for the API key. +#' @param api_key The API key to use. +#' +#' @inherit .shared-request return +#' @keywords internal +.req_auth_api_key_query <- function(req, ..., parameter_name, api_key) { rlang::check_dots_empty() if (length(api_key) && nchar(api_key)) { req <- httr2::req_url_query(req, !!parameter_name := api_key) @@ -56,13 +64,14 @@ security_api_key <- function(req, #' Authenticate with an API key in a cookie #' #' @inheritParams .shared-parameters -#' @param path The path to the cookie. +#' @param file_path The path to the cookie. #' #' @inherit .shared-request return #' @keywords internal -.security_api_key_cookie <- function(req, ..., path) { # nocov start - if (length(path) && nchar(path)) { - req <- httr2::req_cookie_preserve(req, path) +.req_auth_api_key_cookie <- function(req, ..., file_path) { # nocov start + rlang::check_dots_empty() + if (length(file_path) && nchar(file_path)) { + req <- httr2::req_cookie_preserve(req, file_path) } return(req) } # nocov end diff --git a/R/call.R b/R/call.R index 0430432..ff965b2 100644 --- a/R/call.R +++ b/R/call.R @@ -7,7 +7,7 @@ #' [httr2::resp_body_json()]. #' #' @seealso [req_setup()], [req_modify()], [req_perform_opinionated()], -#' [resp_parse()], and [do_if_defined()] for finer control of the process. +#' [resp_parse()], and [do_if_fn_defined()] for finer control of the process. #' #' @inheritParams rlang::args_dots_empty #' @inheritParams req_setup @@ -48,7 +48,7 @@ call_api <- function(base_url, mime_type = mime_type, method = method ) - req <- do_if_defined(req, security_fn, !!!security_args) + req <- do_if_fn_defined(req, security_fn, !!!security_args) resp <- req_perform_opinionated( req, next_req = next_req, diff --git a/R/req.R b/R/req.R index df22221..b61d2b3 100644 --- a/R/req.R +++ b/R/req.R @@ -35,14 +35,10 @@ req_setup <- function(base_url, #' path-specific properties. #' #' @inheritParams rlang::args_dots_empty +#' @inheritParams .req_path_append #' @inheritParams .req_body_auto -#' @inheritParams .shared-parameters -#' @param method If the method is something other than GET or POST, supply it. -#' Case is ignored. -#' @param path The route to an API endpoint. Optionally, a list with the path -#' plus variables to [glue::glue()] into the path. -#' @param query An optional list of parameters to pass in the query portion of -#' the request. +#' @inheritParams .req_method_apply +#' @inheritParams .req_query_flatten #' #' @inherit .shared-request return #' @export @@ -103,18 +99,3 @@ req_prepare <- function(base_url, ) return(req) } - -.req_path_append <- function(req, path) { - if (length(path)) { - path <- rlang::inject(glue::glue(!!!path)) - req <- httr2::req_url_path_append(req, path) - } - return(req) -} - -.req_method_apply <- function(req, method) { - if (length(method)) { - return(httr2::req_method(req, method)) - } - return(req) -} diff --git a/R/req_body.R b/R/req_body.R index 48b1377..68658b5 100644 --- a/R/req_body.R +++ b/R/req_body.R @@ -47,10 +47,7 @@ body, mime_type = NULL) { body <- .prepare_body(body, mime_type) - if (length(body)) { - req <- .add_body(req, body) - } - return(req) + .do_if_args_defined(req, .add_body, body = body) } .add_body <- function(req, body) { diff --git a/R/req_method.R b/R/req_method.R new file mode 100644 index 0000000..2727ceb --- /dev/null +++ b/R/req_method.R @@ -0,0 +1,14 @@ +#' Add a method if it is supplied +#' +#' [httr2::req_method()] errors if `method` is `NULL`, rather than using the +#' default rules. This function deals with that. +#' +#' @inheritParams .shared-parameters +#' @param method If the method is something other than GET or POST, supply it. +#' Case is ignored. +#' +#' @inherit .shared-request return +#' @keywords internal +.req_method_apply <- function(req, method) { + .do_if_args_defined(req, httr2::req_method, method = method) +} diff --git a/R/req_path.R b/R/req_path.R new file mode 100644 index 0000000..584ba5c --- /dev/null +++ b/R/req_path.R @@ -0,0 +1,19 @@ +#' Process a path with glue syntax and append it +#' +#' @inheritParams .shared-parameters +#' @param path The route to an API endpoint. Optionally, a list or character +#' vector with the path as one or more unnamed arguments (which will be +#' concatenated with "/") plus named arguments to [glue::glue()] into the +#' path. +#' +#' @inherit .shared-request return +#' @keywords internal +.req_path_append <- function(req, path) { + .do_if_args_defined(req, .req_path_append_impl, path = path) +} + +.req_path_append_impl <- function(req, path) { + path <- rlang::inject(glue::glue(!!!path, .sep = "/")) + path <- url_normalize(path) + req <- httr2::req_url_path_append(req, path) +} diff --git a/R/req_query.R b/R/req_query.R index 91e389b..921db87 100644 --- a/R/req_query.R +++ b/R/req_query.R @@ -1,9 +1,15 @@ -.req_query_flatten <- function(req, query) { +#' Add non-empty query elements to a request +#' +#' @inheritParams .shared-parameters +#' @param query An optional list or character vector of parameters to pass in +#' the query portion of the request. Can also include a `.multi` argument to +#' pass to [httr2::req_url_query()] to control how elements containing +#' multiple values are handled. +#' +#' @inherit .shared-request return +#' @keywords internal +.req_query_flatten <- function(req, + query) { query <- purrr::discard(query, is.null) - query <- purrr::map_chr(query, .prepare_query_element) - return(httr2::req_url_query(req, !!!query)) -} - -.prepare_query_element <- function(query_element) { - return(paste(unlist(query_element), collapse = ",")) + rlang::inject(httr2::req_url_query(req, !!!query)) } diff --git a/R/resp.R b/R/resp.R index 4b80088..23c04a0 100644 --- a/R/resp.R +++ b/R/resp.R @@ -51,7 +51,7 @@ resp_parse.default <- function(resp, resp_parse.httr2_response <- function(resp, ..., response_parser = httr2::resp_body_json) { - do_if_defined(resp, response_parser, ...) + do_if_fn_defined(resp, response_parser, ...) } #' @export diff --git a/R/utils.R b/R/utils.R index 310e72f..b6a02db 100644 --- a/R/utils.R +++ b/R/utils.R @@ -1,3 +1,5 @@ +# compact_nested_list ---------------------------------------------------------- + #' Discard empty elements #' #' Discard empty elements in nested lists. @@ -35,6 +37,8 @@ compact_nested_list <- function(lst) { return(purrr::compact(lst)) } +# urls ------------------------------------------------------------------------- + #' Add path elements to a URL #' #' Append zero or more path elements to a URL without duplicating "/" @@ -86,6 +90,8 @@ url_normalize <- function(url) { return(sub("/$", "", path)) } +# Do if ------------------------------------------------------------------------ + #' Use a provided function #' #' When constructing API calls programmatically, you may encounter situations @@ -106,7 +112,7 @@ url_normalize <- function(url) { #' build_api_req <- function(endpoint, security_fn = NULL, ...) { #' req <- httr2::request("https://example.com") #' req <- httr2::req_url_path_append(req, endpoint) -#' do_if_defined(req, security_fn, ...) +#' do_if_fn_defined(req, security_fn, ...) #' } #' #' # Most endpoints of this API do not require authentication. @@ -118,7 +124,7 @@ url_normalize <- function(url) { #' "secure_endpoint", httr2::req_auth_bearer_token, "secret-token" #' ) #' secure_req$headers$Authorization -do_if_defined <- function(x, fn = NULL, ...) { +do_if_fn_defined <- function(x, fn = NULL, ...) { if (is.function(fn)) { # Higher-level calls can include !!!'ed arguments. dots <- rlang::list2(...) @@ -126,3 +132,24 @@ do_if_defined <- function(x, fn = NULL, ...) { } return(x) } + +#' Use a function if args are provided +#' +#' @param x An object to potentially modify, such as a [httr2::request()] +#' object. +#' @param fn A function to apply to `x`. If `fn` is `NULL`, `x` is returned +#' unchanged. +#' @param ... Additional arguments to pass to `fn`. +#' +#' @return The object, potentially modified. +#' @keywords internal +.do_if_args_defined <- function(x, fn = NULL, ...) { + if (is.function(fn)) { + dots <- rlang::list2(...) + dots <- purrr::discard(dots, is.null) + if (length(dots)) { + x <- rlang::inject(fn(x, !!!dots)) + } + } + return(x) +} diff --git a/man/call_api.Rd b/man/call_api.Rd index 85e7dec..de53be4 100644 --- a/man/call_api.Rd +++ b/man/call_api.Rd @@ -29,11 +29,15 @@ to choose one.} \item{...}{These dots are for future extensions and must be empty.} -\item{path}{The route to an API endpoint. Optionally, a list with the path -plus variables to \code{\link[glue:glue]{glue::glue()}} into the path.} +\item{path}{The route to an API endpoint. Optionally, a list or character +vector with the path as one or more unnamed arguments (which will be +concatenated with "/") plus named arguments to \code{\link[glue:glue]{glue::glue()}} into the +path.} -\item{query}{An optional list of parameters to pass in the query portion of -the request.} +\item{query}{An optional list or character vector of parameters to pass in +the query portion of the request. Can also include a \code{.multi} argument to +pass to \code{\link[httr2:req_url]{httr2::req_url_query()}} to control how elements containing +multiple values are handled.} \item{body}{An object to use as the body of the request. If any component of the body is a path, pass it through \code{\link[fs:path]{fs::path()}} or otherwise give it the @@ -90,5 +94,5 @@ around the \code{req_} family of functions, such as \code{\link[httr2:request]{h } \seealso{ \code{\link[=req_setup]{req_setup()}}, \code{\link[=req_modify]{req_modify()}}, \code{\link[=req_perform_opinionated]{req_perform_opinionated()}}, -\code{\link[=resp_parse]{resp_parse()}}, and \code{\link[=do_if_defined]{do_if_defined()}} for finer control of the process. +\code{\link[=resp_parse]{resp_parse()}}, and \code{\link[=do_if_fn_defined]{do_if_fn_defined()}} for finer control of the process. } diff --git a/man/do_if_defined.Rd b/man/do_if_fn_defined.Rd similarity index 90% rename from man/do_if_defined.Rd rename to man/do_if_fn_defined.Rd index 24ef28b..aee53ff 100644 --- a/man/do_if_defined.Rd +++ b/man/do_if_fn_defined.Rd @@ -1,10 +1,10 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/utils.R -\name{do_if_defined} -\alias{do_if_defined} +\name{do_if_fn_defined} +\alias{do_if_fn_defined} \title{Use a provided function} \usage{ -do_if_defined(x, fn = NULL, ...) +do_if_fn_defined(x, fn = NULL, ...) } \arguments{ \item{x}{An object to potentially modify, such as a \code{\link[httr2:request]{httr2::request()}} @@ -28,7 +28,7 @@ endpoints. This function exists to make coding such situations easier. build_api_req <- function(endpoint, security_fn = NULL, ...) { req <- httr2::request("https://example.com") req <- httr2::req_url_path_append(req, endpoint) - do_if_defined(req, security_fn, ...) + do_if_fn_defined(req, security_fn, ...) } # Most endpoints of this API do not require authentication. diff --git a/man/dot-do_if_args_defined.Rd b/man/dot-do_if_args_defined.Rd new file mode 100644 index 0000000..10131b3 --- /dev/null +++ b/man/dot-do_if_args_defined.Rd @@ -0,0 +1,24 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{.do_if_args_defined} +\alias{.do_if_args_defined} +\title{Use a function if args are provided} +\usage{ +.do_if_args_defined(x, fn = NULL, ...) +} +\arguments{ +\item{x}{An object to potentially modify, such as a \code{\link[httr2:request]{httr2::request()}} +object.} + +\item{fn}{A function to apply to \code{x}. If \code{fn} is \code{NULL}, \code{x} is returned +unchanged.} + +\item{...}{Additional arguments to pass to \code{fn}.} +} +\value{ +The object, potentially modified. +} +\description{ +Use a function if args are provided +} +\keyword{internal} diff --git a/man/dot-security_api_key_cookie.Rd b/man/dot-req_auth_api_key_cookie.Rd similarity index 62% rename from man/dot-security_api_key_cookie.Rd rename to man/dot-req_auth_api_key_cookie.Rd index e97516d..08e2fc4 100644 --- a/man/dot-security_api_key_cookie.Rd +++ b/man/dot-req_auth_api_key_cookie.Rd @@ -1,15 +1,15 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/security_api_key.R -\name{.security_api_key_cookie} -\alias{.security_api_key_cookie} +% Please edit documentation in R/auth.R +\name{.req_auth_api_key_cookie} +\alias{.req_auth_api_key_cookie} \title{Authenticate with an API key in a cookie} \usage{ -.security_api_key_cookie(req, ..., path) +.req_auth_api_key_cookie(req, ..., file_path) } \arguments{ \item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} -\item{path}{The path to the cookie.} +\item{file_path}{The path to the cookie.} } \value{ A \code{\link[httr2:request]{httr2::request()}} object. diff --git a/man/dot-security_api_key_header.Rd b/man/dot-req_auth_api_key_header.Rd similarity index 72% rename from man/dot-security_api_key_header.Rd rename to man/dot-req_auth_api_key_header.Rd index b2234e8..513ad1f 100644 --- a/man/dot-security_api_key_header.Rd +++ b/man/dot-req_auth_api_key_header.Rd @@ -1,10 +1,10 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/security_api_key.R -\name{.security_api_key_header} -\alias{.security_api_key_header} +% Please edit documentation in R/auth.R +\name{.req_auth_api_key_header} +\alias{.req_auth_api_key_header} \title{Authenticate with an API key in the header of the request} \usage{ -.security_api_key_header(req, ..., parameter_name, api_key) +.req_auth_api_key_header(req, ..., parameter_name, api_key = NULL) } \arguments{ \item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} diff --git a/man/dot-req_auth_api_key_query.Rd b/man/dot-req_auth_api_key_query.Rd new file mode 100644 index 0000000..63de171 --- /dev/null +++ b/man/dot-req_auth_api_key_query.Rd @@ -0,0 +1,22 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/auth.R +\name{.req_auth_api_key_query} +\alias{.req_auth_api_key_query} +\title{Authenticate with an API key in the query of the request} +\usage{ +.req_auth_api_key_query(req, ..., parameter_name, api_key) +} +\arguments{ +\item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} + +\item{parameter_name}{The name to use for the API key.} + +\item{api_key}{The API key to use.} +} +\value{ +A \code{\link[httr2:request]{httr2::request()}} object. +} +\description{ +Authenticate with an API key in the query of the request +} +\keyword{internal} diff --git a/man/dot-req_method_apply.Rd b/man/dot-req_method_apply.Rd new file mode 100644 index 0000000..e01fbfd --- /dev/null +++ b/man/dot-req_method_apply.Rd @@ -0,0 +1,22 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/req_method.R +\name{.req_method_apply} +\alias{.req_method_apply} +\title{Add a method if it is supplied} +\usage{ +.req_method_apply(req, method) +} +\arguments{ +\item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} + +\item{method}{If the method is something other than GET or POST, supply it. +Case is ignored.} +} +\value{ +A \code{\link[httr2:request]{httr2::request()}} object. +} +\description{ +\code{\link[httr2:req_method]{httr2::req_method()}} errors if \code{method} is \code{NULL}, rather than using the +default rules. This function deals with that. +} +\keyword{internal} diff --git a/man/dot-req_path_append.Rd b/man/dot-req_path_append.Rd new file mode 100644 index 0000000..894c1dc --- /dev/null +++ b/man/dot-req_path_append.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/req_path.R +\name{.req_path_append} +\alias{.req_path_append} +\title{Process a path with glue syntax and append it} +\usage{ +.req_path_append(req, path) +} +\arguments{ +\item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} + +\item{path}{The route to an API endpoint. Optionally, a list or character +vector with the path as one or more unnamed arguments (which will be +concatenated with "/") plus named arguments to \code{\link[glue:glue]{glue::glue()}} into the +path.} +} +\value{ +A \code{\link[httr2:request]{httr2::request()}} object. +} +\description{ +Process a path with glue syntax and append it +} +\keyword{internal} diff --git a/man/dot-req_query_flatten.Rd b/man/dot-req_query_flatten.Rd new file mode 100644 index 0000000..7aecf4f --- /dev/null +++ b/man/dot-req_query_flatten.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/req_query.R +\name{.req_query_flatten} +\alias{.req_query_flatten} +\title{Add non-empty query elements to a request} +\usage{ +.req_query_flatten(req, query) +} +\arguments{ +\item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} + +\item{query}{An optional list or character vector of parameters to pass in +the query portion of the request. Can also include a \code{.multi} argument to +pass to \code{\link[httr2:req_url]{httr2::req_url_query()}} to control how elements containing +multiple values are handled.} +} +\value{ +A \code{\link[httr2:request]{httr2::request()}} object. +} +\description{ +Add non-empty query elements to a request +} +\keyword{internal} diff --git a/man/dot-shared-parameters.Rd b/man/dot-shared-parameters.Rd index 97dd0cf..0104bfd 100644 --- a/man/dot-shared-parameters.Rd +++ b/man/dot-shared-parameters.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/aaa_shared.R +% Please edit documentation in R/aaa-shared.R \name{.shared-parameters} \alias{.shared-parameters} \title{Parameters used in multiple functions} diff --git a/man/dot-shared-request.Rd b/man/dot-shared-request.Rd index 807ecf9..94dc594 100644 --- a/man/dot-shared-request.Rd +++ b/man/dot-shared-request.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/aaa_shared.R +% Please edit documentation in R/aaa-shared.R \name{.shared-request} \alias{.shared-request} \title{Returns from request functions} diff --git a/man/security_api_key.Rd b/man/req_auth_api_key.Rd similarity index 85% rename from man/security_api_key.Rd rename to man/req_auth_api_key.Rd index 2454e95..2b07e46 100644 --- a/man/security_api_key.Rd +++ b/man/req_auth_api_key.Rd @@ -1,10 +1,10 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/security_api_key.R -\name{security_api_key} -\alias{security_api_key} +% Please edit documentation in R/auth.R +\name{req_auth_api_key} +\alias{req_auth_api_key} \title{Authenticate with an API key} \usage{ -security_api_key(req, ..., location = "header") +req_auth_api_key(req, ..., location = "header") } \arguments{ \item{req}{A \code{\link[httr2:request]{httr2::request()}} object.} diff --git a/man/req_modify.Rd b/man/req_modify.Rd index a88188a..c0fdca9 100644 --- a/man/req_modify.Rd +++ b/man/req_modify.Rd @@ -19,11 +19,15 @@ req_modify( \item{...}{These dots are for future extensions and must be empty.} -\item{path}{The route to an API endpoint. Optionally, a list with the path -plus variables to \code{\link[glue:glue]{glue::glue()}} into the path.} +\item{path}{The route to an API endpoint. Optionally, a list or character +vector with the path as one or more unnamed arguments (which will be +concatenated with "/") plus named arguments to \code{\link[glue:glue]{glue::glue()}} into the +path.} -\item{query}{An optional list of parameters to pass in the query portion of -the request.} +\item{query}{An optional list or character vector of parameters to pass in +the query portion of the request. Can also include a \code{.multi} argument to +pass to \code{\link[httr2:req_url]{httr2::req_url_query()}} to control how elements containing +multiple values are handled.} \item{body}{An object to use as the body of the request. If any component of the body is a path, pass it through \code{\link[fs:path]{fs::path()}} or otherwise give it the diff --git a/man/req_prepare.Rd b/man/req_prepare.Rd index 6c2358c..7fd3c67 100644 --- a/man/req_prepare.Rd +++ b/man/req_prepare.Rd @@ -22,11 +22,15 @@ to choose one.} \item{...}{These dots are for future extensions and must be empty.} -\item{path}{The route to an API endpoint. Optionally, a list with the path -plus variables to \code{\link[glue:glue]{glue::glue()}} into the path.} +\item{path}{The route to an API endpoint. Optionally, a list or character +vector with the path as one or more unnamed arguments (which will be +concatenated with "/") plus named arguments to \code{\link[glue:glue]{glue::glue()}} into the +path.} -\item{query}{An optional list of parameters to pass in the query portion of -the request.} +\item{query}{An optional list or character vector of parameters to pass in +the query portion of the request. Can also include a \code{.multi} argument to +pass to \code{\link[httr2:req_url]{httr2::req_url_query()}} to control how elements containing +multiple values are handled.} \item{body}{An object to use as the body of the request. If any component of the body is a path, pass it through \code{\link[fs:path]{fs::path()}} or otherwise give it the diff --git a/principles.md b/principles.md new file mode 100644 index 0000000..0bcba36 --- /dev/null +++ b/principles.md @@ -0,0 +1,26 @@ +# nectar design principles + +*This is an experiment in making key package design principles explicit, versus only implicit in the code. The goal is to make maintenance easier, when spread out over time and across people. (borrowed from https://github.com/r-lib/usethis/blob/main/principles.md)* + +## naming + +Function and argument names in this package reflect a tug-of-war between conventions used in [{httr2}](https://httr2.r-lib.org/) and conventions used in the [OpenAPI Specification](https://spec.openapis.org/oas/v3.1.0) (OAS). Use these rules to decide how to name things: + +- Names should be `lower_snake_case`. +- Names should be unique. For example, {httr2} uses `path` in several functions to refer to the URL path, but `req_cookie_preserve()` has a `path` argument for an on-disk location. We use `file_path` for this second argument. +- Internal functions should begin with `.`. This is an older convention, but I find it useful for keeping my mental model of the package clear. If an unexported function is later exported, there will be work required to make sure the function is renamed everywhere, and I consider this a feature, not a bug; it makes it more likely to catch strange circular references and other sticking points. +- If a sub-function (internal) is necessary to keep code clean and readable, but it doesn't do anything unique other than implementing the function that's calling it, append `_impl` (implementation) to the end of the function name. For example, `.req_path_append()` calls `.req_path_append_impl()` when a `path` argument is provided. +- Lean toward the {httr2} conventions for names. If a function modifies a request object, it should start with `req_`, and have an argument `req`. If a function modifies a response object, it should start with `resp_`, and have an argument `resp`. +- If an argument name is confusing in {httr2}, it might be acceptable to rename it. But be very careful about this. See above about unique names, though. +- Names should not overlap with {httr2} function names. A modified `req_perform()` cannot be named `req_perform()`. +- If a function essentially translates from OAS to {httr2}, use OAS names for arguments, and {httr2} names for the function. However... +- Arguments should be syntactic. Instead of `in` (used in [OAS Security Scheme Objects](https://spec.openapis.org/oas/v3.1.0#security-scheme-object)), use `location`. + +## documentation + +- All exported functions must be documented. +- Ideally, internal functions should also be documented, with `@keywords internal`. They do not need a full description, but include a one-line description in the "Title" area, and document their parameters. Most of the time, parameters will be documented in internal functions, and then inherited in the calling exported function (see below). Exception: `_impl()` functions do not need their own documentation; the parent function's documentation covers the details in most or all cases. +- Use `@inheritParams` liberally. Every parameter should be defined in exactly one place. This makes it much easier to maintain definitions. If you add a new parameter, globally search (ctrl-shift-F in RStudio) for `@param {name}` to make sure it isn't already defined. +- If a parameter is reused but doesn't have a clear "home" (eg, `req` is used in several unrelated functions), define it in the `.shared-parameters` block of `aaa-shared.R`. +- Likewise, if a return value is reused but doesn't have a clear "home" (eg the `httr2::request` objects returned by several functions), define it once in `aaa-shared.R` in a block with `@name .shared-{name}`, where `{name}` concisely describes the return value. +- If a choice contradicts with the choice in {httr2}, explain and justify it in documentation. Make sure it is extremely clear when a default is different. diff --git a/tests/testthat/test-auth.R b/tests/testthat/test-auth.R new file mode 100644 index 0000000..3fdaa4e --- /dev/null +++ b/tests/testthat/test-auth.R @@ -0,0 +1,79 @@ +test_that("req_auth_api_key works for header", { + test_result <- req_auth_api_key( + httr2::request("https://example.com"), + parameter_name = "parm", + api_key = "my_key" + ) + expect_in( + test_result$headers, + list(parm = "my_key") + ) + expect_in( + attr(test_result$headers, "redact"), + "parm" + ) + + test_result <- req_auth_api_key( + httr2::request("https://example.com"), + parameter_name = "parm", + api_key = "my_key", + location = "header" + ) + expect_in( + test_result$headers, + list(parm = "my_key") + ) + expect_in( + attr(test_result$headers, "redact"), + "parm" + ) +}) + +test_that("req_auth_api_key works for query", { + test_result <- req_auth_api_key( + httr2::request("https://example.com"), + parameter_name = "parm", + api_key = "my_key", + location = "query" + ) + expect_identical( + test_result$url, + "https://example.com?parm=my_key" + ) +}) + +test_that("req_auth_api_key errors informatively with unused arguments", { + expect_error( + { + req_auth_api_key( + httr2::request("https://example.com"), + location = "header", + api_key = "ok", + file_path = "bad" + ) + }, + class = "rlib_error_dots_nonempty" + ) + expect_error( + { + req_auth_api_key( + httr2::request("https://example.com"), + location = "query", + api_key = "ok", + file_path = "bad" + ) + }, + class = "rlib_error_dots_nonempty" + ) + expect_error( + { + req_auth_api_key( + httr2::request("https://example.com"), + location = "cookie", + api_key = "bad", + file_path = "ok" + ) + }, + class = "rlib_error_dots_nonempty" + ) +}) diff --git a/tests/testthat/test-req_query.R b/tests/testthat/test-req_query.R index 40621c1..d860467 100644 --- a/tests/testthat/test-req_query.R +++ b/tests/testthat/test-req_query.R @@ -13,12 +13,13 @@ test_that("req_prepare() uses query parameters", { ) }) -test_that("req_prepare() smushes & concatenates multi-value query parameters", { +test_that("req_prepare() uses the .multi arg", { test_result <- req_prepare( base_url = "https://example.com", query = list( foo = "bar", - baz = c("qux", "quux") + baz = c("qux", "quux"), + .multi = "comma" ), user_agent = NULL ) diff --git a/tests/testthat/test-security_api_key.R b/tests/testthat/test-security_api_key.R deleted file mode 100644 index d4b168c..0000000 --- a/tests/testthat/test-security_api_key.R +++ /dev/null @@ -1,43 +0,0 @@ -test_that("security_api_key works for header", { - test_result <- security_api_key( - httr2::request("https://example.com"), - parameter_name = "parm", - api_key = "my_key" - ) - expect_in( - test_result$headers, - list(parm = "my_key") - ) - expect_in( - attr(test_result$headers, "redact"), - "parm" - ) - - test_result <- security_api_key( - httr2::request("https://example.com"), - parameter_name = "parm", - api_key = "my_key", - location = "header" - ) - expect_in( - test_result$headers, - list(parm = "my_key") - ) - expect_in( - attr(test_result$headers, "redact"), - "parm" - ) -}) - -test_that("security_api_key works for query", { - test_result <- security_api_key( - httr2::request("https://example.com"), - parameter_name = "parm", - api_key = "my_key", - location = "query" - ) - expect_identical( - test_result$url, - "https://example.com?parm=my_key" - ) -})