diff --git a/NEWS.md b/NEWS.md index 6c8c1ec..ce3b07e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,11 @@ ## Bug Fixes +* `get_by_code()` percent-encodes vocabulary IDs and concept codes as individual + path segments, including literal percent sequences and slashes such as ICD-O + `8032/3`, with or without hierarchy, synonym, relationship, or vocabulary-release + query options. + * HTTP failures now raise the documented OMOPHub condition classes instead of only httr2's generic ones (#137): 400 → `omophub_validation_error`, 401 or `invalid_api_key` / `missing_api_key` → `omophub_auth_error`, 403 → diff --git a/R/concepts.R b/R/concepts.R index d9e16a5..ce37ea4 100644 --- a/R/concepts.R +++ b/R/concepts.R @@ -88,8 +88,14 @@ ConceptsResource <- R6::R6Class( perform_get( private$.base_req, - paste0("concepts/by-code/", vocabulary_id, "/", concept_code), - query = if (length(query) > 0) query else NULL + paste0( + "concepts/by-code/", + utils::URLencode(vocabulary_id, reserved = TRUE, repeated = TRUE), + "/", + utils::URLencode(concept_code, reserved = TRUE, repeated = TRUE) + ), + query = if (length(query) > 0) query else NULL, + endpoint_encoded = TRUE ) }, diff --git a/R/request.R b/R/request.R index 31ccc7a..015a854 100644 --- a/R/request.R +++ b/R/request.R @@ -65,14 +65,18 @@ perform_request <- function(req, endpoint, call = rlang::caller_env()) { #' @param base_req Base request object. #' @param endpoint API endpoint path. #' @param query Named list of query parameters. +#' @param endpoint_encoded Whether `endpoint` is already percent-encoded. #' #' @returns Parsed JSON response. For paginated endpoints, returns a list with #' `data` (the results) and `meta` (pagination info). For single-item endpoints, #' returns the unwrapped data directly. #' @keywords internal -perform_get <- function(base_req, endpoint, query = NULL) { - req <- base_req |> - httr2::req_url_path_append(endpoint) +perform_get <- function(base_req, endpoint, query = NULL, + endpoint_encoded = FALSE) { + req <- base_req + if (!isTRUE(endpoint_encoded)) { + req <- httr2::req_url_path_append(req, endpoint) + } # Add query parameters, removing NULLs if (!is.null(query)) { @@ -82,6 +86,14 @@ perform_get <- function(base_req, endpoint, query = NULL) { } } + if (isTRUE(endpoint_encoded)) { + # Query updates parse and rebuild the URL, decoding escaped path separators. + # Append the already-encoded endpoint only after all query updates are done. + url <- httr2::url_parse(req$url) + url$path <- I(paste0(sub("/$", "", url$path), "/", endpoint)) + req <- httr2::req_url(req, httr2::url_build(url)) + } + resp <- perform_request(req, endpoint, call = rlang::caller_env()) body <- httr2::resp_body_json(resp) diff --git a/man/perform_get.Rd b/man/perform_get.Rd index a0f852e..dd8754c 100644 --- a/man/perform_get.Rd +++ b/man/perform_get.Rd @@ -4,7 +4,7 @@ \alias{perform_get} \title{Perform GET Request} \usage{ -perform_get(base_req, endpoint, query = NULL) +perform_get(base_req, endpoint, query = NULL, endpoint_encoded = FALSE) } \arguments{ \item{base_req}{Base request object.} @@ -12,6 +12,8 @@ perform_get(base_req, endpoint, query = NULL) \item{endpoint}{API endpoint path.} \item{query}{Named list of query parameters.} + +\item{endpoint_encoded}{Whether \code{endpoint} is already percent-encoded.} } \value{ Parsed JSON response. For paginated endpoints, returns a list with diff --git a/tests/testthat/test-concepts.R b/tests/testthat/test-concepts.R index 1fd636d..e1f0aed 100644 --- a/tests/testthat/test-concepts.R +++ b/tests/testthat/test-concepts.R @@ -108,7 +108,7 @@ test_that("concepts$get_by_code calls correct endpoint", { called_with <- NULL local_mocked_bindings( - perform_get = function(req, path, query = NULL) { + perform_get = function(req, path, query = NULL, endpoint_encoded = FALSE) { called_with <<- list(path = path) mock_concept() } @@ -119,13 +119,35 @@ test_that("concepts$get_by_code calls correct endpoint", { expect_equal(called_with$path, "concepts/by-code/SNOMED/44054006") }) +test_that("concepts$get_by_code URL-encodes path segments", { + base_req <- httr2::request("https://api.omophub.com/v1") + resource <- ConceptsResource$new(base_req) + + request_url <- NULL + httr2::local_mocked_responses(function(req) { + request_url <<- req$url + httr2::response( + status_code = 200, + headers = list(`content-type` = "application/json"), + body = charToRaw(jsonlite::toJSON(mock_concept(), auto_unbox = TRUE)) + ) + }) + + resource$get_by_code("ICDO3", "8032/3") + + expect_equal( + request_url, + "https://api.omophub.com/v1/concepts/by-code/ICDO3/8032%2F3" + ) +}) + test_that("concepts$get_by_code passes include_hierarchy and vocab_release", { base_req <- httr2::request("https://api.omophub.com/v1") resource <- ConceptsResource$new(base_req) called_with <- NULL local_mocked_bindings( - perform_get = function(req, path, query = NULL) { + perform_get = function(req, path, query = NULL, endpoint_encoded = FALSE) { called_with <<- list(query = query) mock_concept() } @@ -469,3 +491,62 @@ test_that("concepts$recommended passes all optional filters", { expect_equal(called_with$body$page, 2L) expect_equal(called_with$body$page_size, 50L) }) + +test_that("concepts$get_by_code preserves encoded slashes with query options", { + base_req <- httr2::request("https://api.omophub.com/v1") + resource <- ConceptsResource$new(base_req) + request_url <- NULL + httr2::local_mocked_responses(function(req) { + request_url <<- req$url + httr2::response( + status_code = 200, + headers = list(`content-type` = "application/json"), + body = charToRaw(jsonlite::toJSON(mock_concept(), auto_unbox = TRUE)) + ) + }) + + resource$get_by_code("ICDO3", "8032/3", include_hierarchy = TRUE) + expect_identical( + request_url, + "https://api.omophub.com/v1/concepts/by-code/ICDO3/8032%2F3?include_hierarchy=true" + ) + + resource$get_by_code( + "ICDO3", "8032/3", + include_relationships = TRUE, + include_synonyms = TRUE, + include_hierarchy = TRUE, + vocab_release = "2025.1" + ) + expect_identical( + strsplit(request_url, "?", fixed = TRUE)[[1]][1], + "https://api.omophub.com/v1/concepts/by-code/ICDO3/8032%2F3" + ) + expect_equal(httr2::url_parse(request_url)$query, list( + include_relationships = "true", + include_synonyms = "true", + include_hierarchy = "true", + vocab_release = "2025.1" + )) +}) + +test_that("concepts$get_by_code treats percent sequences as literal segment input", { + resource <- ConceptsResource$new(httr2::request("https://api.omophub.com/v1")) + request_url <- NULL + httr2::local_mocked_responses(function(req) { + request_url <<- req$url + httr2::response( + status_code = 200, + headers = list(`content-type` = "application/json"), + body = charToRaw(jsonlite::toJSON(mock_concept(), auto_unbox = TRUE)) + ) + }) + + for (include_hierarchy in c(FALSE, TRUE)) { + resource$get_by_code("V%20B/1", "A%20B/1", include_hierarchy = include_hierarchy) + expect_identical( + strsplit(request_url, "?", fixed = TRUE)[[1]][1], + "https://api.omophub.com/v1/concepts/by-code/V%2520B%2F1/A%2520B%2F1" + ) + } +})