diff --git a/R/config.R b/R/config.R index 6ebc78f..a5534b1 100644 --- a/R/config.R +++ b/R/config.R @@ -21,7 +21,30 @@ .uscogdata_defaults[[key]] } -.resolve_url <- function() .cfg("url") +#' Resolve the corpus URL, guaranteeing the trailing slash the package assumes. +#' +#' Every consumer builds locations by CONCATENATION -- `paste0(url, +#' "manifest.json")` in manifest.R, `paste0(url, e$path)` in mirror.R, and the +#' parquet glob in views.R -- and mirror.R:104 documents the invariant outright +#' ('url ends in "/"'). Nothing enforced it, so a URL entered without the slash +#' failed silently and misleadingly: +#' +#' HTTPS -> ".../downloadmanifest.json"; the host answers with an HTML 404 +#' page, which lands in the JSON parser as the lexical error +#' reported in issue #3 -- pointing the user at "login page / wrong +#' share" when the real cause was one missing character. +#' local -> ".../corpusdata/long/**/*.parquet" and a DuckDB "No files found". +#' +#' Normalizing here fixes every consumer at once, rather than each call site +#' re-deriving the same invariant. An empty setting is passed through +#' untouched so manifest.R's "not configured" guard still fires instead of the +#' value degrading into a bare "/" filesystem root. +#' @noRd +.resolve_url <- function() { + url <- .cfg("url") + if (is.null(url) || !nzchar(url) || grepl("/$", url)) return(url) + paste0(url, "/") +} .resolve_cache_dir <- function() { v <- .cfg("cache_dir") diff --git a/tests/testthat/test-config.R b/tests/testthat/test-config.R index 8356046..3461ab2 100644 --- a/tests/testthat/test-config.R +++ b/tests/testthat/test-config.R @@ -26,3 +26,41 @@ test_that(".resolve_cache_dir falls back to R_user_dir", { }) }) }) + +# --------------------------------------------------------------------------- +# Trailing-slash normalization (uscogdata #3 follow-up). +# +# EVERY consumer builds paths by concatenation: paste0(url, "manifest.json") +# (manifest.R), paste0(url, e$path) (mirror.R), and the parquet glob in +# views.R. mirror.R:104 even comments 'url ends in "/"' -- an assumption the +# package documents and relies on but never enforced. +# +# A URL missing its trailing slash therefore fails SILENTLY and confusingly: +# HTTPS -> ".../downloadmanifest.json" -> the host answers with an HTML 404 +# page -> the jsonlite lexical error that issue #3 reported; +# local -> ".../corpusdata/long/**/*.parquet" -> DuckDB "No files found". +# Neither message points at the real cause. Normalize once, at resolution. +# --------------------------------------------------------------------------- + +test_that(".resolve_url appends a missing trailing slash", { + withr::local_envvar(USCOGDATA_URL = "https://example.org/s/TOKEN/download") + expect_equal(.resolve_url(), "https://example.org/s/TOKEN/download/") +}) + +test_that(".resolve_url leaves an existing trailing slash alone", { + withr::local_envvar(USCOGDATA_URL = "https://example.org/s/TOKEN/download/") + expect_equal(.resolve_url(), "https://example.org/s/TOKEN/download/") +}) + +test_that(".resolve_url normalizes a local path without a trailing slash", { + withr::local_envvar(USCOGDATA_URL = "/tmp/corpus") + expect_equal(.resolve_url(), "/tmp/corpus/") +}) + +test_that(".resolve_url does not invent a slash for an empty setting", { + # An unset/empty URL must stay empty so the "not configured" guard in + # manifest.R still fires, rather than degrading into a bare "/" root. + withr::local_envvar(USCOGDATA_URL = "") + withr::local_options(uscogdata.url = "") + expect_equal(.resolve_url(), "") +})