gitlabr
gitlabr copied to clipboard
Inconsistent handling of `file_path` arg in convenience functions wrt percent-encoding
Consider the following reprex:
gitlabr::set_gitlab_connection(gitlab_con = gitlabr::gl_project_connection(gitlab_url = "https://gitlab.com",
project = 19126559L,
private_token = Sys.getenv("GITLAB_COM_TOKEN")))
path_rel <- "inst/rstudio/addins.dcf"
# `gl_get_file()` fails without percent-encoded `file_path`
try(
gitlabr::gl_get_file(file_path = path_rel,
ref = "master")
)
#> Error in http_error_or_content(.) : Not Found (HTTP 404).
gitlabr::gl_get_file(file_path = xml2::url_escape(path_rel),
ref = "master")
#> [1] "Name: Process R Markdown package\nDescription: Executes `pkgpurl::process_pkg()`.\nBinding: process_pkg\nInteractive: true\n\nName: Load R Markdown package\nDescription: Executes `pkgpurl::load_pkg()`.\nBinding: load_pkg\nInteractive: true\n\nName: Purl `Rmd/`\nDescription: Executes `pkgpurl::purl_rmd()`.\nBinding: purl_rmd\nInteractive: false\n\nName: Lint R Markdown package\nDescription: Executes `pkgpurl::lint_rmd()`.\nBinding: lint_rmd\nInteractive: true\n\nName: Run `*.nopurl.Rmd` files\nDescription: Executes `pkgpurl::run_nopurl_rmd()`.\nBinding: run_nopurl_rmd\nInteractive: true\n"
# `gl_file_exists()` OTOH doesn't work properly with percent-encoded `file_path`
gitlabr::gl_file_exists(file_path = xml2::url_escape(path_rel),
ref = "master")
#> [1] FALSE
gitlabr::gl_file_exists(file_path = path_rel,
ref = "master")
#> [1] TRUE
Created on 2024-01-17 with reprex v2.1.0
As can be seen in the example above, gl_get_file() requires the file_path argument to be percent-encoded while gl_file_exists() requires the opposite. This is confusing. I think gitlabr should instead expect the file_path / path arguments to be unescaped, i.e. literal, in all functions and do the necessary escaping internally. This would be more convenient and less error-prone for users.
Thanks for reporting.
This is what we started to fix in this PR : https://github.com/ThinkR-open/gitlabr/pull/106 , and I hardly find time to finish this big step...
Note that this problem also causes gitlabr::gl_push_file() to fail updating existing files in subfolders since it uses gl_file_exists() internally
https://github.com/ThinkR-open/gitlabr/blob/bd31578710289034fe03bd654bd0899a669625f1/R/files.R#L127
and thus wrongly concludes an already existing file wouldn't exist and makes a POST instead of a PUT request.
( gl_push_file() requires file_path to be already percent-encoded (same as gl_get_file()), while gl_file_exists() requires the opposite.)
That should be good now in the dev version.
I treated this one in PR: https://github.com/ThinkR-open/gitlabr/pull/106
And added some unit tests to verify.
Thank you for your exploration