Skip to content

Commit

Permalink
Revert URL subtraction
Browse files Browse the repository at this point in the history
It appears we merged the URL subtraction a bit too soon, and we need
to rework the approach. I know have ~20 hours into it trying to solve
the issues. The conclusion is that it is not ready to ship and we need
a new approach.

To ensure it does not go out before its ready, revert the original PR
and the attempts to solve the issues with it so we can start fresh.

References
  • Loading branch information
bdraco committed Oct 25, 2024
1 parent 6a2a90a commit ccc4d34
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 193 deletions.
22 changes: 0 additions & 22 deletions CHANGES/1340.feature.rst

This file was deleted.

15 changes: 0 additions & 15 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -997,21 +997,6 @@ The path is encoded if needed.
>>> base.join(URL('//python.org/page.html'))
URL('http://python.org/page.html')

The subtraction (``-``) operator creates a new URL with
a relative *path* to the target URL from the given base URL.
*scheme*, *user*, *password*, *host*, *port*, *query* and *fragment* are removed.

.. method:: URL.__sub__(url)

Returns a new URL with a relative *path* between two other URL objects.

.. doctest::

>>> target = URL('http://example.com/path/index.html')
>>> base = URL('http://example.com/')
>>> target - base
URL('path/index.html')

Human readable representation
-----------------------------

Expand Down
83 changes: 0 additions & 83 deletions tests/test_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,89 +61,6 @@ def test_str():
assert str(url) == "http://example.com:8888/path/to?a=1&b=2"


@pytest.mark.parametrize(
("target", "base", "expected"),
[
("http://example.com/path/to", "http://example.com/", "path/to"),
("http://example.com/path/to", "http://example.com/spam", "path/to"),
("http://example.com/this/is/a/test", "http://example.com/this/", "is/a/test"),
(
"http://example.com/this/./is/a/test",
"http://example.com/this/",
"is/a/test",
),
("http://example.com/this/is/../a//test", "http://example.com/this/", "a/test"),
("http://example.com/path/to", "http://example.com/spam/", "../path/to"),
("http://example.com/path", "http://example.com/path/to/", ".."),
("http://example.com/path", "http://example.com/other/../path/to/", ".."),
("http://example.com/", "http://example.com/", "."),
("http://example.com", "http://example.com", "."),
("http://example.com/", "http://example.com", "."),
("http://example.com", "http://example.com/", "."),
("//example.com", "//example.com", "."),
("/path/to", "/spam/", "../path/to"),
("path/to", "spam/", "../path/to"),
("path/../to", "path/", "../to"),
("path/..", ".", "path/.."),
("path/../replace/me", "path/../replace", "replace/me"),
("path/../replace/me", "path/../replace/", "me"),
("path/to", "spam", "path/to"),
("..", ".", ".."),
(".", "..", "."),
],
)
def test_sub(target: str, base: str, expected: str):
expected_url = URL(expected)
result_url = URL(target) - URL(base)
assert result_url == expected_url


@pytest.mark.xfail(reason="Empty segments are not preserved")
@pytest.mark.parametrize(
("target", "base", "expected"),
[
(
"http://example.com/////path/////to",
"http://example.com/////spam",
"path/////to",
),
(
"http://example.com////path/////to",
"http://example.com/////spam",
"..//path/////to",
),
],
)
def test_sub_empty_segments(target: str, base: str, expected: str):
expected_url = URL(expected)
result_url = URL(target) - URL(base)
assert result_url == expected_url


def test_sub_with_different_schemes():
expected_error_msg = "Both URLs should have the same scheme"
with pytest.raises(ValueError, match=expected_error_msg):
URL("http://example.com/") - URL("https://example.com/")


def test_sub_with_different_netlocs():
expected_error_msg = "Both URLs should have the same netloc"
with pytest.raises(ValueError, match=expected_error_msg):
URL("https://spam.com/") - URL("https://ham.com/")


def test_sub_with_different_anchors():
expected_error_msg = "'path/to' and '/path' have different anchors"
with pytest.raises(ValueError, match=expected_error_msg):
URL("path/to") - URL("/path/from")


def test_sub_with_two_dots_in_base():
expected_error_msg = "'..' segment in '/path/..' cannot be walked"
with pytest.raises(ValueError, match=expected_error_msg):
URL("path/to") - URL("/path/../from")


def test_repr():
url = URL("http://example.com")
assert "URL('http://example.com')" == repr(url)
Expand Down
14 changes: 0 additions & 14 deletions tests/test_url_benchmarks.py
Original file line number Diff line number Diff line change
Expand Up @@ -589,20 +589,6 @@ def _run() -> None:
url.query


def test_url_subtract(benchmark: BenchmarkFixture) -> None:
@benchmark
def _run() -> None:
for _ in range(100):
URL_WITH_LONGER_PATH - URL_WITH_PATH


def test_url_subtract_long_urls(benchmark: BenchmarkFixture) -> None:
@benchmark
def _run() -> None:
for _ in range(100):
URL_VERY_LONG_PATH - URL_LONG_PATH


def test_url_host_port_subcomponent(benchmark: BenchmarkFixture) -> None:
cache_non_default = URL_WITH_NOT_DEFAULT_PORT._cache
cache = BASE_URL._cache
Expand Down
30 changes: 0 additions & 30 deletions yarl/_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from collections.abc import Sequence
from contextlib import suppress
from itertools import chain
from pathlib import PurePosixPath


def normalize_path_segments(segments: Sequence[str]) -> list[str]:
Expand Down Expand Up @@ -41,31 +39,3 @@ def normalize_path(path: str) -> str:

segments = path.split("/")
return prefix + "/".join(normalize_path_segments(segments))


def calculate_relative_path(target: str, base: str) -> str:
"""Return the relative path between two other paths.
If the operation is not possible, raise ValueError.
"""

target = target or "/"
base = base or "/"

target_path = PurePosixPath(target)
base_path = PurePosixPath(base)

if base[-1] != "/":
base_path = base_path.parent

for step, path in enumerate(chain((base_path,), base_path.parents)):
if path == target_path or path in target_path.parents:
break
elif path.name == "..":
raise ValueError(f"'..' segment in {str(base_path)!r} cannot be walked")
else:
raise ValueError(
f"{str(target_path)!r} and {str(base_path)!r} have different anchors"
)
offset = len(path.parts)
return str(PurePosixPath(*("..",) * step, *target_path.parts[offset:]))
30 changes: 1 addition & 29 deletions yarl/_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from propcache.api import under_cached_property as cached_property

from ._parse import USES_AUTHORITY, make_netloc, split_netloc, split_url, unsplit_result
from ._path import calculate_relative_path, normalize_path, normalize_path_segments
from ._path import normalize_path, normalize_path_segments
from ._query import (
Query,
QueryVariable,
Expand Down Expand Up @@ -477,34 +477,6 @@ def __truediv__(self, name: str) -> "URL":
return NotImplemented
return self._make_child((str(name),))

def __sub__(self, other: object) -> "URL":
"""Return a new URL with a relative path between two other URL objects.
Note that both URLs must have the same scheme and netloc.
The new relative URL has only path:
scheme, user, password, host, port, query and fragment are removed.
Example:
>>> target = URL("http://example.com/path/index.html")
>>> base = URL("http://example.com/")
>>> target - base
URL('path/index.html')
"""

if type(other) is not URL:
return NotImplemented

target_scheme, target_netloc, target_path, _, _ = self._val
base_scheme, base_netloc, base_path, _, _ = other._val

if target_scheme != base_scheme:
raise ValueError("Both URLs should have the same scheme")
if target_netloc != base_netloc:
raise ValueError("Both URLs should have the same netloc")

path = calculate_relative_path(target_path, base_path)
return self._from_tup(("", "", path, "", ""))

def __mod__(self, query: Query) -> "URL":
return self.update_query(query)

Expand Down

0 comments on commit ccc4d34

Please sign in to comment.