use a nice dataclass to decouple hash parsing from Link

Author: cosmicexplorer

src/pip/_internal/commands/download.py

@@ -11,65 +11,32 @@
 from pip._internal.cli.cmdoptions import make_target_python
 from pip._internal.cli.req_command import RequirementCommand, with_cleanup
 from pip._internal.cli.status_codes import SUCCESS
-from pip._internal.models.link import Link
+from pip._internal.models.link import Link, LinkHash
 from pip._internal.req.req_tracker import get_requirement_tracker
 from pip._internal.utils.misc import ensure_dir, normalize_path, write_output
 from pip._internal.utils.temp_dir import TempDirectory
 
 logger = logging.getLogger(__name__)
 
 
-@dataclass
-class RequirementHash:
-    hash_name: str
-    hash_value: str
-
-    @classmethod
-    def from_dist_info_metadata(
-        cls,
-        dist_info_metadata: Optional[str],
-    ) -> Optional["RequirementHash"]:
-        if dist_info_metadata is None:
-            return None
-        if dist_info_metadata == "true":
-            return None
-        # FIXME: don't use private `_hash_re`!
-        hash_match = Link._hash_re.match(dist_info_metadata)
-        if hash_match is None:
-            return None
-        hash_name, hash_value = hash_match.groups()
-        return cls(hash_name=hash_name, hash_value=hash_value)
-
-    @classmethod
-    def from_link(cls, link: Link) -> Optional["RequirementHash"]:
-        if not link.is_hash_allowed:
-            return None
-        hash_name = link.hash_name
-        hash_value = link.hash
-        assert hash_name is not None
-        assert hash_value is not None
-        return cls(hash_name=hash_name, hash_value=hash_value)
-
-    def as_json(self) -> Dict[str, str]:
-        return {
-            "hash_name": self.hash_name,
-            "hash_value": self.hash_value,
-        }
-
-
-@dataclass
+@dataclass(frozen=True)
 class DistInfoMetadata:
     """???/From PEP 658"""
 
     metadata_url: str
-    metadata_hash: Optional[RequirementHash]
+    metadata_hash: Optional[LinkHash]
 
     @classmethod
     def from_link(cls, link: Link) -> Optional["DistInfoMetadata"]:
         if link.dist_info_metadata is None:
             return None
+
         metadata_url = f"{link.url_without_fragment}.metadata"
-        metadata_hash = RequirementHash.from_dist_info_metadata(link.dist_info_metadata)
+        if link.dist_info_metadata == "true":
+            metadata_hash = None
+        else:
+            metadata_hash = LinkHash.split_hash_name_and_value(link.dist_info_metadata)
+
         return cls(metadata_url=metadata_url, metadata_hash=metadata_hash)
 
     def as_json(self) -> Dict[str, Union[str, Optional[Dict[str, str]]]]:
@@ -81,11 +48,11 @@ def as_json(self) -> Dict[str, Union[str, Optional[Dict[str, str]]]]:
         }
 
 
-@dataclass
+@dataclass(frozen=True)
 class RequirementDownloadInfo:
     req: Requirement
     url: str
-    file_hash: Optional[RequirementHash]
+    file_hash: Optional[LinkHash]
     dist_info_metadata: Optional[DistInfoMetadata]
 
     @classmethod
@@ -97,7 +64,7 @@ def from_req_and_link(
         return cls(
             req=req,
             url=link.url,
-            file_hash=RequirementHash.from_link(link),
+            file_hash=link.get_link_hash(),
             dist_info_metadata=DistInfoMetadata.from_link(link),
         )
 

src/pip/_internal/models/link.py

@@ -6,6 +6,7 @@
 import re
 import urllib.parse
 import xml.etree.ElementTree
+from dataclasses import dataclass
 from typing import TYPE_CHECKING, Dict, List, NamedTuple, Optional, Tuple, Union
 
 from pip._internal.utils.filetypes import WHEEL_EXTENSION
@@ -31,6 +32,42 @@
 _SUPPORTED_HASHES = ("sha1", "sha224", "sha384", "sha256", "sha512", "md5")
 
 
+# FIXME: test this!!
+@dataclass(frozen=True)
+class LinkHash:
+    name: str
+    value: str
+
+    # TODO: consider beginning/ending this with \b?
+    # TODO: consider re.IGNORECASE?
+    _hash_re = re.compile(
+        r"({choices})=([a-f0-9]+)".format(choices="|".join(_SUPPORTED_HASHES))
+    )
+
+    @classmethod
+    @functools.lru_cache(maxsize=None)
+    def split_hash_name_and_value(cls, url: str) -> Optional["LinkHash"]:
+        match = cls._hash_re.search(url)
+        if match is None:
+            return None
+        name, value = match.groups()
+        return cls(name=name, value=value)
+
+    def is_hash_allowed(self, hashes: Optional[Hashes]) -> bool:
+        """
+        Return True if the current hash is allowed by `hashes`.
+        """
+        if hashes is None:
+            return False
+        return hashes.is_hash_allowed(self.name, hex_digest=self.value)
+
+    def as_json(self) -> Dict[str, str]:
+        return {
+            "name": self.name,
+            "value": self.value,
+        }
+
+
 def _clean_url_path_part(part: str) -> str:
     """
     Clean a "part" of a URL path (i.e. after splitting on "@" characters).
@@ -266,22 +303,21 @@ def subdirectory_fragment(self) -> Optional[str]:
             return None
         return match.group(1)
 
-    _hash_re = re.compile(
-        r"({choices})=([a-f0-9]+)".format(choices="|".join(_SUPPORTED_HASHES))
-    )
+    def get_link_hash(self) -> Optional[LinkHash]:
+        return LinkHash.split_hash_name_and_value(self._url)
 
     @property
     def hash(self) -> Optional[str]:
-        match = self._hash_re.search(self._url)
-        if match:
-            return match.group(2)
+        link_hash = self.get_link_hash()
+        if link_hash is not None:
+            return link_hash.value
         return None
 
     @property
     def hash_name(self) -> Optional[str]:
-        match = self._hash_re.search(self._url)
-        if match:
-            return match.group(1)
+        link_hash = self.get_link_hash()
+        if link_hash is not None:
+            return link_hash.name
         return None
 
     @property
@@ -311,19 +347,16 @@ def is_yanked(self) -> bool:
 
     @property
     def has_hash(self) -> bool:
-        return self.hash_name is not None
+        return self.get_link_hash() is not None
 
     def is_hash_allowed(self, hashes: Optional[Hashes]) -> bool:
         """
-        Return True if the link has a hash and it is allowed.
+        Return True if the link has a hash and it is allowed by `hashes`.
         """
-        if hashes is None or not self.has_hash:
+        link_hash = self.get_link_hash()
+        if link_hash is None:
             return False
-        # Assert non-None so mypy knows self.hash_name and self.hash are str.
-        assert self.hash_name is not None
-        assert self.hash is not None
-
-        return hashes.is_hash_allowed(self.hash_name, hex_digest=self.hash)
+        return link_hash.is_hash_allowed(hashes)
 
 
 class _CleanResult(NamedTuple):

tests/unit/test_collector.py

@@ -2,6 +2,7 @@
 import logging
 import os.path
 import re
+import urllib.parse
 import urllib.request
 import uuid
 from textwrap import dedent
@@ -15,8 +16,6 @@
 from pip._internal.index.collector import (
     HTMLPage,
     LinkCollector,
-    _clean_link,
-    _clean_url_path,
     _determine_base_url,
     _get_html_page,
     _get_html_response,
@@ -28,7 +27,7 @@
 from pip._internal.index.sources import _FlatDirectorySource, _IndexDirectorySource
 from pip._internal.models.candidate import InstallationCandidate
 from pip._internal.models.index import PyPI
-from pip._internal.models.link import Link
+from pip._internal.models.link import Link, _clean_link, _clean_url_path
 from pip._internal.network.session import PipSession
 from tests.lib import TestData, make_test_link_collector
 from tests.lib.path import Path
@@ -415,7 +414,7 @@ def test_clean_url_path_with_local_path(path: str, expected: str) -> None:
     ],
 )
 def test_clean_link(url: str, clean_url: str) -> None:
-    assert _clean_link(url) == clean_url
+    assert _clean_link(Link(url)).parsed == urllib.parse.urlsplit(clean_url)
 
 
 def _test_parse_links_data_attribute(