Merge origin/main (PR #8: corpus URL trailing-slash normalization) into feat/expenditure-concept
Local main was stale atfa40266when this branch was created, so it was missing748ca4a. Merging rather than rebasing to preserve the reviewed commit SHAs recorded in the SDD ledger.
This commit is contained in:
+24
-1
@@ -21,7 +21,30 @@
|
|||||||
.uscogdata_defaults[[key]]
|
.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() {
|
.resolve_cache_dir <- function() {
|
||||||
v <- .cfg("cache_dir")
|
v <- .cfg("cache_dir")
|
||||||
|
|||||||
@@ -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(), "")
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user