From a38f82d609dbe2a209a3323d4cff6e6bbfe85f1e Mon Sep 17 00:00:00 2001 From: Alex Chen <238273288+alex-omophub@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:49:42 +0100 Subject: [PATCH 1/2] fix: encode concept lookup path segments --- R/concepts.R | 7 ++++++- tests/testthat/test-concepts.R | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/R/concepts.R b/R/concepts.R index d9e16a5..9b9543b 100644 --- a/R/concepts.R +++ b/R/concepts.R @@ -88,7 +88,12 @@ ConceptsResource <- R6::R6Class( perform_get( private$.base_req, - paste0("concepts/by-code/", vocabulary_id, "/", concept_code), + paste0( + "concepts/by-code/", + utils::URLencode(vocabulary_id, reserved = TRUE), + "/", + utils::URLencode(concept_code, reserved = TRUE) + ), query = if (length(query) > 0) query else NULL ) }, diff --git a/tests/testthat/test-concepts.R b/tests/testthat/test-concepts.R index 1fd636d..934702b 100644 --- a/tests/testthat/test-concepts.R +++ b/tests/testthat/test-concepts.R @@ -119,6 +119,23 @@ 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) + + called_with <- NULL + local_mocked_bindings( + perform_get = function(req, path, query = NULL) { + called_with <<- list(path = path) + mock_concept() + } + ) + + resource$get_by_code("ICDO3", "8032/3") + + expect_equal(called_with$path, "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) From 36d326eb785b68821afdaa5e9ac886f676cd1734 Mon Sep 17 00:00:00 2001 From: Alex Chen <238273288+alex-omophub@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:07:36 +0100 Subject: [PATCH 2/2] test: verify encoded concept path on request --- R/concepts.R | 3 ++- R/request.R | 14 +++++++++++--- man/perform_get.Rd | 4 +++- tests/testthat/test-concepts.R | 25 +++++++++++++++---------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/R/concepts.R b/R/concepts.R index 9b9543b..7faf462 100644 --- a/R/concepts.R +++ b/R/concepts.R @@ -94,7 +94,8 @@ ConceptsResource <- R6::R6Class( "/", utils::URLencode(concept_code, reserved = TRUE) ), - query = if (length(query) > 0) query else NULL + query = if (length(query) > 0) query else NULL, + endpoint_encoded = TRUE ) }, diff --git a/R/request.R b/R/request.R index 3859149..7ab9ed5 100644 --- a/R/request.R +++ b/R/request.R @@ -44,14 +44,22 @@ build_request <- function(base_url, api_key, timeout = 30, max_retries = 3, #' @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) { + if (isTRUE(endpoint_encoded)) { + url <- httr2::url_parse(base_req$url) + url$path <- I(paste0(sub("/$", "", url$path), "/", endpoint)) + req <- httr2::req_url(base_req, httr2::url_build(url)) + } else { + req <- base_req |> + httr2::req_url_path_append(endpoint) + } # Add query parameters, removing NULLs if (!is.null(query)) { 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 934702b..4b38677 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() } @@ -123,17 +123,22 @@ test_that("concepts$get_by_code URL-encodes path segments", { 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) { - called_with <<- list(path = path) - mock_concept() - } - ) + 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(called_with$path, "concepts/by-code/ICDO3/8032%2F3") + 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", { @@ -142,7 +147,7 @@ test_that("concepts$get_by_code passes include_hierarchy and vocab_release", { 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() }