Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/httpx2/httpx2/_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,32 @@ def __init__(self, url: URL | str = "", **kwargs: typing.Any) -> None:
kwargs[key] = value.decode("ascii")

if "params" in kwargs:
# Replace any "params" keyword with the raw "query" instead.
# Merge any "params" keyword with the URL's existing "query"
# string, so that calling `URL("https://x?a=1", params={"b": 2})`
# keeps the original query string and appends new params.
#
# Ensure that empty params use `kwargs["query"] = None` rather
# than `kwargs["query"] = ""`, so that generated URLs do not
# include an empty trailing "?".
# `params` takes precedence on key collisions (matches
# `requests` and `URL.copy_merge_params` semantics).
params = kwargs.pop("params")
kwargs["query"] = None if not params else str(QueryParams(params))
if params:
# Defer the merge until after the URL has been parsed, so
# we can read the existing query string off the parsed URL.
kwargs["__merge_params__"] = params

merge_params = kwargs.pop("__merge_params__", None)
if isinstance(url, str):
self._uri_reference = urlparse(url, **kwargs)
elif isinstance(url, URL):
self._uri_reference = url._uri_reference.copy_with(**kwargs)
else:
raise TypeError(f"Invalid type for url. Expected str or httpx2.URL, got {type(url)}: {url!r}")
if merge_params:
# Now that the URL is parsed, read its current query and merge the
# new params on top. Re-parse so the new query becomes canonical.
existing_query = self._uri_reference.query
existing = QueryParams(existing_query) if existing_query else QueryParams()
merged = existing.merge(merge_params)
self._uri_reference = self._uri_reference.copy_with(query=str(merged))

@property
def scheme(self) -> str:
Expand Down Expand Up @@ -343,7 +355,11 @@ def copy_add_param(self, key: str, value: typing.Any = None) -> URL:
return self.copy_with(params=self.params.add(key, value))

def copy_remove_param(self, key: str) -> URL:
return self.copy_with(params=self.params.remove(key))
# With the new merge semantics of the URL/Request constructor, passing
# `params=self.params.remove(key)` would re-merge the (now-empty) params
# with the existing query string and silently leave the param in place.
# Reset the query string to None so the param is actually removed.
return self.copy_with(query=None, params=self.params.remove(key))

def copy_merge_params(self, params: QueryParamTypes) -> URL:
return self.copy_with(params=self.params.merge(params))
Expand Down
6 changes: 4 additions & 2 deletions tests/httpx2/models/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,9 @@ def test_request_params() -> None:
assert str(request.url) == "http://example.com"

request = httpx2.Request("GET", "http://example.com?c=3", params={"a": "1", "b": "2"})
assert str(request.url) == "http://example.com?a=1&b=2"
assert str(request.url) == "http://example.com?c=3&a=1&b=2"

# `params={}` no longer wipes the URL's existing query string — merge
# semantics now apply in both directions.
request = httpx2.Request("GET", "http://example.com?a=1", params={})
assert str(request.url) == "http://example.com"
assert str(request.url) == "http://example.com?a=1"
4 changes: 2 additions & 2 deletions tests/httpx2/models/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ def test_url_params() -> None:
assert url.params == httpx2.QueryParams({"a": "123"})

url = httpx2.URL("https://example.org:123/path/to/somewhere?b=456", params={"a": "123"})
assert str(url) == "https://example.org:123/path/to/somewhere?a=123"
assert url.params == httpx2.QueryParams({"a": "123"})
assert str(url) == "https://example.org:123/path/to/somewhere?b=456&a=123"
assert url.params == httpx2.QueryParams({"a": "123", "b": "456"})


# Tests for username and password
Expand Down
62 changes: 62 additions & 0 deletions tests/httpx2/test_url_params_merge.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"""
Tests for the URL constructor: `URL(url, params=...)` should merge the new
params with the URL's existing query string instead of replacing it.
Regression for issue #905.
"""

from __future__ import annotations

import httpx2


def test_url_constructor_params_merges_with_existing_query() -> None:
"""`URL("https://example.com/?a=1", params={"b": 2})` should keep `a=1`."""
url = httpx2.URL("https://example.com/?a=1", params={"b": 2})
assert url.params["a"] == "1"
assert url.params["b"] == "2"


def test_url_constructor_params_keeps_order_existing_first() -> None:
url = httpx2.URL("https://example.com/?a=1&b=2", params={"c": 3})
assert str(url) == "https://example.com/?a=1&b=2&c=3"


def test_url_constructor_params_overrides_existing_on_key_collision() -> None:
"""When the same key is given, the explicit `params` value wins."""
url = httpx2.URL("https://example.com/?a=1", params={"a": "2"})
assert url.params["a"] == "2"


def test_url_constructor_no_params_keeps_existing_query() -> None:
"""No `params` keyword, existing query is preserved (sanity)."""
url = httpx2.URL("https://example.com/?a=1&b=2")
assert url.params["a"] == "1"
assert url.params["b"] == "2"


def test_url_constructor_empty_params_keeps_existing_query() -> None:
"""Empty `params` should not wipe out the URL's query string."""
url = httpx2.URL("https://example.com/?a=1", params={})
assert url.params["a"] == "1"


def test_url_constructor_no_existing_query_just_uses_params() -> None:
url = httpx2.URL("https://example.com/", params={"a": "1", "b": "2"})
assert url.params["a"] == "1"
assert url.params["b"] == "2"


def test_get_request_merges_query_with_params() -> None:
"""End-to-end: `httpx2.get(url, params={...})` should concatenate."""
# We don't make a network call — we only check the request that would be sent.
transport = httpx2.MockTransport(lambda req: httpx2.Response(200, json={"ok": True}))
with httpx2.Client(transport=transport) as client:
req = client.build_request(
"GET",
"https://httpbin.org/get?page=post&s=list",
params={"pid": 0, "tags": "k-on!"},
)
assert "page=post" in str(req.url)
assert "s=list" in str(req.url)
assert "pid=0" in str(req.url)
assert "tags=k-on" in str(req.url) # `!` is percent-encoded
Loading