performance: cache pages of PyPI repository in the same way as for legacy repository to fix performance regression caused by replacing cachy

Author: radoering

Diff for: src/poetry/repositories/http_repository.py

@@ -30,8 +30,11 @@
 
 
 if TYPE_CHECKING:
+    from packaging.utils import NormalizedName
+
     from poetry.config.config import Config
     from poetry.inspection.info import PackageInfo
+    from poetry.repositories.link_sources.base import LinkSource
     from poetry.utils.authenticator import RepositoryCertificateConfig
 
 
@@ -293,8 +296,8 @@ def _get_response(self, endpoint: str) -> requests.Response | None:
             )
         return response
 
-    def _get_page(self, endpoint: str) -> HTMLPage | None:
-        response = self._get_response(endpoint)
+    def _get_page(self, name: NormalizedName) -> LinkSource | None:
+        response = self._get_response(f"/{name}/")
         if not response:
             return None
         return HTMLPage(response.url, response.text)

Diff for: src/poetry/repositories/legacy_repository.py

@@ -72,7 +72,7 @@ def package(
             return package
 
     def find_links_for_package(self, package: Package) -> list[Link]:
-        page = self.get_page(f"/{package.name}/")
+        page = self.get_page(package.name)
         if page is None:
             return []
 
@@ -90,12 +90,9 @@ def _find_packages(
         if not constraint.is_any():
             key = f"{key}:{constraint!s}"
 
-        page = self.get_page(f"/{name}/")
+        page = self.get_page(name)
         if page is None:
-            self._log(
-                f"No packages found for {name}",
-                level="debug",
-            )
+            self._log(f"No packages found for {name}", level="debug")
             return []
 
         versions = [
@@ -119,7 +116,7 @@ def _find_packages(
     def _get_release_info(
         self, name: NormalizedName, version: Version
     ) -> dict[str, Any]:
-        page = self.get_page(f"/{name}/")
+        page = self.get_page(name)
         if page is None:
             raise PackageNotFound(f'No package named "{name}"')
 
@@ -141,8 +138,8 @@ def _get_release_info(
             ),
         )
 
-    def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
-        response = self._get_response(endpoint)
+    def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage | None:
+        response = self._get_response(f"/{name}/")
         if not response:
             return None
         return SimpleRepositoryPage(response.url, response.text)

Diff for: src/poetry/repositories/pypi_repository.py

@@ -111,13 +111,11 @@ def _find_packages(
         Find packages on the remote server.
         """
         try:
-            json_page = self.get_json_page(name)
+            json_page = self.get_page(name)
         except PackageNotFound:
-            self._log(
-                f"No packages found for {name}",
-                level="debug",
-            )
+            self._log(f"No packages found for {name}", level="debug")
             return []
+        assert isinstance(json_page, SimpleJsonPage)
 
         versions: list[tuple[Version, str | bool]]
 
@@ -226,7 +224,7 @@ def _get_release_info(
 
         return data.asdict()
 
-    def get_json_page(self, name: NormalizedName) -> SimpleJsonPage:
+    def _get_page(self, name: NormalizedName) -> SimpleJsonPage:
         source = self._base_url + f"simple/{name}/"
         info = self.get_package_info(name)
         return SimpleJsonPage(source, info)

Diff for: src/poetry/repositories/single_page_repository.py

@@ -1,11 +1,17 @@
 from __future__ import annotations
 
+from typing import TYPE_CHECKING
+
 from poetry.repositories.legacy_repository import LegacyRepository
 from poetry.repositories.link_sources.html import SimpleRepositoryPage
 
 
+if TYPE_CHECKING:
+    from packaging.utils import NormalizedName
+
+
 class SinglePageRepository(LegacyRepository):
-    def _get_page(self, endpoint: str | None = None) -> SimpleRepositoryPage | None:
+    def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage | None:
         """
         Single page repositories only have one page irrespective of endpoint.
         """

Diff for: tests/installation/test_chooser.py

@@ -98,7 +98,7 @@ def callback(
         parts = uri.rsplit("/")
         name = parts[-2]
 
-        fixture = LEGACY_FIXTURES / (name + "_partial_yank" + ".html")
+        fixture = LEGACY_FIXTURES / (name + "-partial-yank" + ".html")
 
         with fixture.open(encoding="utf-8") as f:
             return [200, headers, f.read()]

Diff for: tests/repositories/fixtures/legacy/futures_partial_yank.html renamed to tests/repositories/fixtures/legacy/futures-partial-yank.html

@@ -30,6 +30,7 @@
     import httpretty
 
     from _pytest.monkeypatch import MonkeyPatch
+    from packaging.utils import NormalizedName
 
     from poetry.config.config import Config
 
@@ -45,16 +46,13 @@ class MockRepository(LegacyRepository):
     def __init__(self) -> None:
         super().__init__("legacy", url="http://legacy.foo.bar", disable_cache=True)
 
-    def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
-        parts = endpoint.split("/")
-        name = parts[1]
-
+    def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage | None:
         fixture = self.FIXTURES / (name + ".html")
         if not fixture.exists():
             return None
 
         with fixture.open(encoding="utf-8") as f:
-            return SimpleRepositoryPage(self._url + endpoint, f.read())
+            return SimpleRepositoryPage(self._url + f"/{name}/", f.read())
 
     def _download(self, url: str, dest: Path) -> None:
         filename = urlparse.urlparse(url).path.rsplit("/")[-1]
@@ -73,7 +71,7 @@ def test_packages_property_returns_empty_list() -> None:
 def test_page_relative_links_path_are_correct() -> None:
     repo = MockRepository()
 
-    page = repo.get_page("/relative")
+    page = repo.get_page("relative")
     assert page is not None
 
     for link in page.links:
@@ -84,7 +82,7 @@ def test_page_relative_links_path_are_correct() -> None:
 def test_page_absolute_links_path_are_correct() -> None:
     repo = MockRepository()
 
-    page = repo.get_page("/absolute")
+    page = repo.get_page("absolute")
     assert page is not None
 
     for link in page.links:
@@ -95,7 +93,7 @@ def test_page_absolute_links_path_are_correct() -> None:
 def test_page_clean_link() -> None:
     repo = MockRepository()
 
-    page = repo.get_page("/relative")
+    page = repo.get_page("relative")
     assert page is not None
 
     cleaned = page.clean_link('https://legacy.foo.bar/test /the"/cleaning\0')
@@ -105,7 +103,7 @@ def test_page_clean_link() -> None:
 def test_page_invalid_version_link() -> None:
     repo = MockRepository()
 
-    page = repo.get_page("/invalid-version")
+    page = repo.get_page("invalid-version")
     assert page is not None
 
     links = list(page.links)
@@ -123,7 +121,7 @@ def test_page_invalid_version_link() -> None:
 
 def test_sdist_format_support() -> None:
     repo = MockRepository()
-    page = repo.get_page("/relative")
+    page = repo.get_page("relative")
     assert page is not None
     bz2_links = list(filter(lambda link: link.ext == ".tar.bz2", page.links))
     assert len(bz2_links) == 1
@@ -434,8 +432,8 @@ def test_package_yanked(
 
 def test_package_partial_yank():
     class SpecialMockRepository(MockRepository):
-        def _get_page(self, endpoint: str) -> SimpleRepositoryPage | None:
-            return super()._get_page(f"/{endpoint.strip('/')}_partial_yank/")
+        def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage | None:
+            return super()._get_page(canonicalize_name(f"{name}-partial-yank"))
 
     repo = MockRepository()
     package = repo.package("futures", Version.parse("3.2.0"))
@@ -481,31 +479,31 @@ def __init__(
 
 
 def test_get_200_returns_page(http: type[httpretty.httpretty]) -> None:
-    repo = MockHttpRepository({"/foo": 200}, http)
+    repo = MockHttpRepository({"/foo/": 200}, http)
 
-    assert repo.get_page("/foo")
+    assert repo.get_page("foo")
 
 
 @pytest.mark.parametrize("status_code", [401, 403, 404])
 def test_get_40x_and_returns_none(
     http: type[httpretty.httpretty], status_code: int
 ) -> None:
-    repo = MockHttpRepository({"/foo": status_code}, http)
+    repo = MockHttpRepository({"/foo/": status_code}, http)
 
-    assert repo.get_page("/foo") is None
+    assert repo.get_page("foo") is None
 
 
 def test_get_5xx_raises(http: type[httpretty.httpretty]) -> None:
-    repo = MockHttpRepository({"/foo": 500}, http)
+    repo = MockHttpRepository({"/foo/": 500}, http)
 
     with pytest.raises(RepositoryError):
-        repo.get_page("/foo")
+        repo.get_page("foo")
 
 
 def test_get_redirected_response_url(
     http: type[httpretty.httpretty], monkeypatch: MonkeyPatch
 ) -> None:
-    repo = MockHttpRepository({"/foo": 200}, http)
+    repo = MockHttpRepository({"/foo/": 200}, http)
     redirect_url = "http://legacy.redirect.bar"
 
     def get_mock(
@@ -517,7 +515,7 @@ def get_mock(
         return response
 
     monkeypatch.setattr(repo.session, "get", get_mock)
-    page = repo.get_page("/foo")
+    page = repo.get_page("foo")
     assert page is not None
     assert page._url == "http://legacy.redirect.bar/foo/"
 

Diff for: tests/repositories/test_legacy_repository.py

@@ -3,13 +3,18 @@
 import re
 
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 from poetry.core.packages.dependency import Dependency
 
 from poetry.repositories.link_sources.html import SimpleRepositoryPage
 from poetry.repositories.single_page_repository import SinglePageRepository
 
 
+if TYPE_CHECKING:
+    from packaging.utils import NormalizedName
+
+
 class MockSinglePageRepository(SinglePageRepository):
     FIXTURES = Path(__file__).parent / "fixtures" / "single-page"
 
@@ -20,7 +25,7 @@ def __init__(self, page: str) -> None:
             disable_cache=True,
         )
 
-    def _get_page(self, endpoint: str = None) -> SimpleRepositoryPage | None:
+    def _get_page(self, name: NormalizedName) -> SimpleRepositoryPage | None:
         fixture = self.FIXTURES / self.url.rsplit("/", 1)[-1]
         if not fixture.exists():
             return