fix(#3): normalize the corpus URL trailing slash at resolution #8

Merged
jared merged 1 commits from fix/3-url-trailing-slash into main 2026-07-25 19:03:12 -04:00
2 changed files with 62 additions and 1 deletions
Showing only changes of commit 748ca4a56e - Show all commits
+24 -1
View File
@@ -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")
+38
View File
@@ -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(), "")
})