http icon indicating copy to clipboard operation
http copied to clipboard

Cannot change path of Uri with less than two failure branches

Open WhyNotHugo opened this issue 2 years ago • 3 comments

I have a many scenarios where I have one Uri and want to change its path_and_query, but this doesn't seem possible without writing code that can yield at least two distinct errors.

This doesn't make sense; realistically, only one error can occur: that the path_and_query is invalid. Beyond that, the operation should be infallible.

Here's one approach that I tried:

    // self.base_url is of type `Uri`
    pub fn relative_uri(&self, path: &str) -> Result<Uri, XXX> {
        let mut parts = self.base_url.clone().into_parts();
        parts.path_and_query = Some(path.try_into().unwrap());
        Uri::from_parts(parts).unwrap()
    }

Because there's two possible error types here (http::uri::InvalidUriParts or http::uri::InvalidUri), I need to have an enum error type for this function that can be either of both variants (or Box it, or wrap it in some common type).

This makes my API a mess because now callers need to handle two distinct error types (or a container of various types) for an operation that, realistically, can only fail in a single way (the path_and_query supplied is invalid).

I tried using the Builder pattern, but the type returned by hyper::Uri::scheme cannot passed to http::uri::Builder::scheme, so I end up with a bunch of failure branches again.

These three approaches could work around the issue:

  • Mutate an Uri and change its path_and_query, or
  • Create a new Uri using the Parts from another while changing only one part.
  • Allow creating a builder pre-populated with Parts from an existing Uri. This would allow re-writing the above example in a way that only one error needs to be handled.

The first one has been mentioned in the past and it seems its undesirable, so I'll just ignore it. The second is tricky to design with a clean API. The third is probably the simplest, since the builder can be initialized with the parts from the source Uri.

I think something like this is quite viable:

impl Builder {
    // ...
    pub fn from_uri(uri: Uri) -> Self {
        Builder {
            parts: Ok(uri.into_parts()),
        }
    }

This could then be used as:

        Builder::from_uri(my_uri)
            .path_and_query(path)
            .build()
            .unwrap();

Note that there's only one failure branch (and one error type) that needs to be handled.

Though, perhaps the following is as simple, useful and a bit more correct:

impl Builder {
    // ...
    pub fn from_parts(parts: Parts) -> Self {
        Builder {
            parts: Ok(parts),
        }
    }

WhyNotHugo avatar Mar 06 '23 16:03 WhyNotHugo

There should be setters. UriBuilder should have From<Uri> implemented

blueforesticarus avatar Nov 21 '24 04:11 blueforesticarus

        let uri = http::uri::Builder::from(req.uri().clone())
            .path_and_query(
                parent_path
                    + &req
                        .uri()
                        .query()
                        .map(|s| format!("?{}", s))
                        .unwrap_or_default(),
            )
            .build()
            .unwrap();
        *req.uri_mut() = uri;

this is the deranged what is wrong with req.uri.path = "adf" ?

blueforesticarus avatar Nov 21 '24 05:11 blueforesticarus

what is wrong with req.uri.path = "adf" ?

This approach would not allow any validation at all.

WhyNotHugo avatar Dec 09 '24 16:12 WhyNotHugo