refactor: update url join method implementation and function signature

Author: Meetesh-Saini

python/pydantic_core/_pydantic_core.pyi

@@ -606,13 +606,13 @@ class Url(SupportsAllComparisons):
             An instance of URL
         """
 
-    def join(self, path: str, trailing_slash: bool = True) -> Self:
+    def join(self, path: str, append_trailing_slash: bool = False) -> Self:
         """
         Parse a string `path` as an URL, using this URL as the base.
 
         Args:
             path: The string (typically a relative URL) to parse and join with the base URL.
-            trailing_slash: Whether to append a trailing slash at the end of the URL.
+            append_trailing_slash: Whether to append a trailing slash at the end of the URL.
 
         Returns:
             A new `Url` instance

src/url.rs

@@ -199,23 +199,24 @@ impl PyUrl {
         cls.call1((url,))
     }
 
-    #[pyo3(signature=(path, trailing_slash=true))]
-    pub fn join(&self, path: &str, trailing_slash: bool) -> PyResult<Self> {
+    #[pyo3(signature=(path, append_trailing_slash=false))]
+    pub fn join(&self, path: &str, append_trailing_slash: bool) -> PyResult<Self> {
         let mut new_url = self
             .lib_url
             .join(path)
             .map_err(|err| PyValueError::new_err(err.to_string()))?;
 
-        if !trailing_slash || new_url.query().is_some() || new_url.fragment().is_some() || new_url.cannot_be_a_base() {
-            return Ok(PyUrl::new(new_url));
-        }
-
-        new_url
-            .path_segments_mut()
-            .map_err(|()| PyValueError::new_err("Url cannot be a base"))?
-            .pop_if_empty()
-            .push("");
+        if append_trailing_slash && !(new_url.query().is_some() || new_url.fragment().is_some()) {
+            let path_segments_result = new_url.path_segments_mut().map(|mut segments| {
+                segments.pop_if_empty().push("");
+            });
 
+            if path_segments_result.is_err() {
+                let mut new_path = new_url.path().to_string();
+                new_path.push('/');
+                new_url.set_path(&new_path);
+            }
+        }
         Ok(PyUrl::new(new_url))
     }
 }

tests/validators/test_url.py

@@ -1324,7 +1324,7 @@ def test_url_build() -> None:
         ('http://a/b/c/d/e', '/../../f/g/', 'http://a/f/g/', 'http://a/f/g/'),
         ('http://a/b/c/d/e/', '../../f/g', 'http://a/b/c/f/g/', 'http://a/b/c/f/g'),
         ('http://a/b/', '../../f/g/', 'http://a/f/g/', 'http://a/f/g/'),
-        (SIMPLE_BASE, 'g:h', 'g:h', 'g:h'),
+        (SIMPLE_BASE, 'g:h', 'g:h/', 'g:h'),
         (SIMPLE_BASE, 'g', 'http://a/b/c/g/', 'http://a/b/c/g'),
         (SIMPLE_BASE, './g', 'http://a/b/c/g/', 'http://a/b/c/g'),
         (SIMPLE_BASE, 'g/', 'http://a/b/c/g/', 'http://a/b/c/g/'),
@@ -1350,7 +1350,7 @@ def test_url_build() -> None:
         (SIMPLE_BASE + '/', 'foo', SIMPLE_BASE + '/foo/', SIMPLE_BASE + '/foo'),
         (QUERY_BASE, '?y', 'http://a/b/c/d;p?y', 'http://a/b/c/d;p?y'),
         (QUERY_BASE, ';x', 'http://a/b/c/;x/', 'http://a/b/c/;x'),
-        (QUERY_BASE, 'g:h', 'g:h', 'g:h'),
+        (QUERY_BASE, 'g:h', 'g:h/', 'g:h'),
         (QUERY_BASE, 'g', 'http://a/b/c/g/', 'http://a/b/c/g'),
         (QUERY_BASE, './g', 'http://a/b/c/g/', 'http://a/b/c/g'),
         (QUERY_BASE, 'g/', 'http://a/b/c/g/', 'http://a/b/c/g/'),
@@ -1436,8 +1436,8 @@ def test_url_join(base_url, join_path, expected_with_slash, expected_without_sla
     and the URL specification from https://url.spec.whatwg.org/
     """
     url = Url(base_url)
-    assert str(url.join(join_path, trailing_slash=True)) == expected_with_slash
-    assert str(url.join(join_path, trailing_slash=False)) == expected_without_slash
+    assert str(url.join(join_path, append_trailing_slash=True)) == expected_with_slash
+    assert str(url.join(join_path, append_trailing_slash=False)) == expected_without_slash
 
 
 def test_url_join_operators() -> None: