Soft deprecate req_url_path_*()

[hadley]

Since req_url_relative() is a more general interface. And improve req_url() docs, links, and examples.

Part of #625

@jonthegeek what do you think of this? I'm wondering if it's too aggressive and maybe I should only deprecate req_url_path_append().

[jonthegeek]

@hadley I'm definitely concerned, but I haven't played with the new modifiers yet. I regularly use req_url_path_append() to "add" a particular endpoint onto a base request. The relative stuff MOSTLY handles that, but if my api is at https://noisyurl.com/apis/rest/analytics/v17/<endpoint>, I want to make sure I can add on /endpoint without accidentally replacing /apis/rest/analytics/v17. I'll take a look at the new setup tonight or tomorrow morning and see if it "feels" like these are really deprecatable.

[jonthegeek]

I don't think I'll use req_url_relative(). You need to note whether the base URL of the API includes any path elements, so it feels more burdensome to me than req_url_path_append(). It turns out req_template() can 100% replace my usual use-case (without having to include the method in the template), though, so I might switch to that:

library(httr2)
library(waldo)
# In my real use-case, this also includes auth + a special `Linkedin-Version`
# header required by LinkedIn.
li_base <- httr2::request("https://api.linkedin.com/rest")

# In pretty much all of my {httr2} code, I then append a path to the base
# request, eg:
current <- li_base |> 
  httr2::req_url_path_append("posts")

# This loses part of the "base" request url.
relative <- li_base |> 
  httr2::req_url_relative("posts")
waldo::compare(current$url, relative$url)
#> `old`: "https://api.linkedin.com/rest/posts"
#> `new`: "https://api.linkedin.com/posts"

# This technically works but I didn't expect it to from the docs. I prefer to
# keep method + path abstracted as separate steps, but this lets me get pretty
# close.
template <- li_base |> 
  httr2::req_template("/posts")
waldo::compare(current$url, template$url)
#> ✔ No differences

^{Created on 2025-01-06 with reprex v2.1.1}

I think I'd prefer something like maybe req_endpoint() (or req_path() since OpenAPI calls that specific part "paths", but I understand that it gets a bit overloaded with filesystem stuff). But req_template() as an option that CAN include glue syntax and CAN include method if that's how I have the docs feels like it might be a place for me to focus.

So, for this particular PR... probably fine? But I don't think it will have the expected effect on my code.

[hadley]

You could use req_url_relative() if you use https://api.linkedin.com/rest/ as the base url - the trailing slash is important here. (But maybe that's generally too subtle for folks?)

[hadley]

Regardless, I'm convinced that this is too aggressive, so I won't do it.

[jonthegeek]

You could use req_url_relative() if you use https://api.linkedin.com/rest/ as the base url - the trailing slash is important here. (But maybe that's generally too subtle for folks?)

Aha! It seemed like it SHOULD work, so I'm glad there's a way. I feel like it's easy to ignore that distinction (or at least I hope it is, since I missed it!), particularly since that slash so often doesn't mean anything in the browser (for example, https://github.com/jonthegeek and https://github.com/jonthegeek/ go to the same place). I assume there's good reason for those to be seen as different by curl_parse_url(), though.

Oh, but this is also seen as different:

li_base <- httr2::request("https://api.linkedin.com/rest/")
current <- li_base |> 
  httr2::req_url_path_append("posts")

relative2 <- li_base |> 
  httr2::req_url_relative("/posts")
waldo::compare(current$url, relative2$url)
#> `old`: "https://api.linkedin.com/rest/posts"
#> `new`: "https://api.linkedin.com/posts" 

That completely makes sense for req_url_relative(), but, since APIs can be inconsistent about whether they document things with path or /path, I like that req_url_path_append() deals with // when it stitches things together:

current <- li_base |> 
  httr2::req_url_path_append("posts")
current2 <- li_base |> 
  httr2::req_url_path_append("/posts")
waldo::compare(current$url, current2$url)
#> ✔ No differences

req_template() also deals with merging the /'s, but looking under the hood, it looks like that's just because it's calling req_url_path_append() at the end, so switching to that would just add some unnecessary processing steps.

I like the idea of req_url_relative(), but I feel like it's too / fragile, so I don't think I'd switch to it. For example, before this conversation, I'd have expected this to go to /rest/other, but you need to include the trailing / to do that:

li_base <- httr2::request("https://api.linkedin.com/rest/")
li_base |> 
  httr2::req_url_path_append("/posts") |> 
  httr2::req_url_relative("../other") |> 
  _$url
#> [1] "https://api.linkedin.com/other"

li_base |> 
  httr2::req_url_path_append("/posts/") |> 
  httr2::req_url_relative("../other") |> 
  _$url
#> [1] "https://api.linkedin.com/rest/other"

I think I'm expecting it to work more like {fs}:

fs::path("mydir") |> 
  fs::path("another") |> 
  fs::path("../other") |> 
  fs::path_norm() # to get it to collapse the ..
#> mydir/other

I think ultimately my issue is with curl::curl_parse_url() (and thus probably with the libcurl URL parser). This seems incorrect to me:

curl::curl_parse_url("../other", "https://myurl.com/rest/posts")$url
#> [1] "https://myurl.com/other"