Repository: john-kurkowski/tldextract

Issue:
Incorrectly parsing unicode dot
Domain angelinablog。com.de  is parsed as:
- Domain: angelinablog。com
- Suffix: de

It should be parsed as:
- angelinablog 
- com 
- de

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 22a642b..57c6cb8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,22 @@
 After upgrading, update your cache file by deleting it or via `tldextract
 --update`.
 
+## 3.2.1 (2022-04-11)
+
+* Bugfixes
+  * Fix incorrect namespace used for caching function returns ([#258](https://github.com/john-kurkowski/tldextract/issues/258))
+  * Remove redundant encode ([`6e2c0e0`](https://github.com/john-kurkowski/tldextract/commit/6e2c0e0))
+  * Remove redundant lowercase ([`226bfc2`](https://github.com/john-kurkowski/tldextract/commit/226bfc2))
+  * Remove unused `try`/`except` path ([#255](https://github.com/john-kurkowski/tldextract/issues/255))
+  * Add types to the private API (disallow untyped calls and defs) ([#256](https://github.com/john-kurkowski/tldextract/issues/256))
+  * Rely on `python_requires` instead of runtime check ([#247](https://github.com/john-kurkowski/tldextract/issues/247))
+* Docs
+  * Fix docs with updated types
+  * Fix link in Travis CI badge ([#248](https://github.com/john-kurkowski/tldextract/issues/248))
+  * Rewrite documentation intro
+  * Remove unnecessary subheading
+  * Unify case
+
 ## 3.2.0 (2022-02-20)
 
 * Features
diff --git a/README.md b/README.md
index 7b6fef9..0e37f42 100644
--- a/README.md
+++ b/README.md
@@ -124,7 +124,7 @@ custom_cache_extract = tldextract.TLDExtract(cache_dir='/path/to/your/cache/')
 custom_cache_extract('http://www.google.com')
 
 # extract callable that doesn't use caching
-no_cache_extract = tldextract.TLDExtract(cache_dir=False)
+no_cache_extract = tldextract.TLDExtract(cache_dir=None)
 no_cache_extract('http://www.google.com')
 ```
 
diff --git a/setup.cfg b/setup.cfg
index 0c9e0fc..852dfff 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,2 +1,10 @@
 [metadata]
 license_file = LICENSE
+
+[mypy]
+check_untyped_defs = True
+disallow_incomplete_defs = True
+disallow_untyped_calls = True
+
+[mypy-tldextract.*]
+disallow_untyped_defs = True
diff --git a/tldextract/cache.py b/tldextract/cache.py
index 8a66488..fbd02c9 100644
--- a/tldextract/cache.py
+++ b/tldextract/cache.py
@@ -7,13 +7,17 @@ import os
 import os.path
 import sys
 from hashlib import md5
+from typing import Callable, Dict, Hashable, Iterable, Optional, TypeVar, Union
 
 from filelock import FileLock
+import requests
 
 LOG = logging.getLogger(__name__)
 
 _DID_LOG_UNABLE_TO_CACHE = False
 
+T = TypeVar("T")  # pylint: disable=invalid-name
+
 
 def get_pkg_unique_identifier() -> str:
     """
@@ -75,7 +79,7 @@ def get_cache_dir() -> str:
 class DiskCache:
     """Disk _cache that only works for jsonable values"""
 
-    def __init__(self, cache_dir, lock_timeout=20):
+    def __init__(self, cache_dir: Optional[str], lock_timeout: int = 20):
         self.enabled = bool(cache_dir)
         self.cache_dir = os.path.expanduser(str(cache_dir) or "")
         self.lock_timeout = lock_timeout
@@ -83,7 +87,7 @@ class DiskCache:
         # combined with a call to `.clear()` wont wipe someones hard drive
         self.file_ext = ".tldextract.json"
 
-    def get(self, namespace, key):
+    def get(self, namespace: str, key: Union[str, Dict[str, Hashable]]) -> T:
         """Retrieve a value from the disk cache"""
         if not self.enabled:
             raise KeyError("Cache is disabled")
@@ -99,10 +103,13 @@ class DiskCache:
             LOG.error("error reading TLD cache file %s: %s", cache_filepath, exc)
             raise KeyError("namespace: " + namespace + " key: " + repr(key)) from None
 
-    def set(self, namespace, key, value):
+    def set(
+        self, namespace: str, key: Union[str, Dict[str, Hashable]], value: T
+    ) -> None:
         """Set a value in the disk cache"""
         if not self.enabled:
-            return False
+            return
+
         cache_filepath = self._key_to_cachefile_path(namespace, key)
 
         try:
@@ -118,7 +125,7 @@ class DiskCache:
                         "unable to cache %s.%s in %s. This could refresh the "
                         "Public Suffix List over HTTP every app startup. "
                         "Construct your `TLDExtract` with a writable `cache_dir` or "
-                        "set `cache_dir=False` to silence this warning. %s"
+                        "set `cache_dir=None` to silence this warning. %s"
                     ),
                     namespace,
                     key,
@@ -127,9 +134,7 @@ class DiskCache:
                 )
                 _DID_LOG_UNABLE_TO_CACHE = True
 
-        return None
-
-    def clear(self):
+    def clear(self) -> None:
         """Clear the disk cache"""
         for root, _, files in os.walk(self.cache_dir):
             for filename in files:
@@ -146,7 +151,9 @@ class DiskCache:
                         if exc.errno != errno.ENOENT:
                             raise
 
-    def _key_to_cachefile_path(self, namespace, key):
+    def _key_to_cachefile_path(
+        self, namespace: str, key: Union[str, Dict[str, Hashable]]
+    ) -> str:
         namespace_path = os.path.join(self.cache_dir, namespace)
         hashed_key = _make_cache_key(key)
 
@@ -154,7 +161,13 @@ class DiskCache:
 
         return cache_path
 
-    def run_and_cache(self, func, namespace, kwargs, hashed_argnames):
+    def run_and_cache(
+        self,
+        func: Callable[..., T],
+        namespace: str,
+        kwargs: Dict[str, Hashable],
+        hashed_argnames: Iterable[str],
+    ) -> T:
         """Get a url but cache the response"""
         if not self.enabled:
             return func(**kwargs)
@@ -172,7 +185,7 @@ class DiskCache:
                         "unable to cache %s.%s in %s. This could refresh the "
                         "Public Suffix List over HTTP every app startup. "
                         "Construct your `TLDExtract` with a writable `cache_dir` or "
-                        "set `cache_dir=False` to silence this warning. %s"
+                        "set `cache_dir=None` to silence this warning. %s"
                     ),
                     namespace,
                     key_args,
@@ -187,14 +200,16 @@ class DiskCache:
         # pylint: disable-next=abstract-class-instantiated
         with FileLock(lock_path, timeout=self.lock_timeout):
             try:
-                result = self.get(namespace=namespace, key=key_args)
+                result: T = self.get(namespace=namespace, key=key_args)
             except KeyError:
                 result = func(**kwargs)
-                self.set(namespace="urls", key=key_args, value=result)
+                self.set(namespace=namespace, key=key_args, value=result)
 
             return result
 
-    def cached_fetch_url(self, session, url, timeout):
+    def cached_fetch_url(
+        self, session: requests.Session, url: str, timeout: Union[float, int, None]
+    ) -> str:
         """Get a url but cache the response"""
         return self.run_and_cache(
             func=_fetch_url,
@@ -204,8 +219,7 @@ class DiskCache:
         )
 
 
-def _fetch_url(session, url, timeout):
-
+def _fetch_url(session: requests.Session, url: str, timeout: Optional[int]) -> str:
     response = session.get(url, timeout=timeout)
     response.raise_for_status()
     text = response.text
@@ -216,16 +230,12 @@ def _fetch_url(session, url, timeout):
     return text
 
 
-def _make_cache_key(inputs):
+def _make_cache_key(inputs: Union[str, Dict[str, Hashable]]) -> str:
     key = repr(inputs)
-    try:
-        key = md5(key).hexdigest()
-    except TypeError:
-        key = md5(key.encode("utf8")).hexdigest()
-    return key
+    return md5(key.encode("utf8")).hexdigest()
 
 
-def _make_dir(filename):
+def _make_dir(filename: str) -> None:
     """Make a directory if it doesn't already exist"""
     if not os.path.exists(os.path.dirname(filename)):
         try:
diff --git a/tldextract/cli.py b/tldextract/cli.py
index 0d4f043..4aceaf8 100644
--- a/tldextract/cli.py
+++ b/tldextract/cli.py
@@ -9,7 +9,7 @@ from ._version import version as __version__
 from .tldextract import TLDExtract
 
 
-def main():
+def main() -> None:
     """tldextract CLI main command."""
     logging.basicConfig()
 
diff --git a/tldextract/remote.py b/tldextract/remote.py
index fcb5c52..1e896ce 100644
--- a/tldextract/remote.py
+++ b/tldextract/remote.py
@@ -1,4 +1,4 @@
-'tldextract helpers for testing and fetching remote resources.'
+"tldextract helpers for testing and fetching remote resources."
 
 import re
 import socket
@@ -7,12 +7,14 @@ from urllib.parse import scheme_chars
 
 
 IP_RE = re.compile(
-    r'^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$')  # pylint: disable=line-too-long
+    # pylint: disable-next=line-too-long
+    r"^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$"
+)
 
-SCHEME_RE = re.compile(r'^([' + scheme_chars + ']+:)?//')
+SCHEME_RE = re.compile(r"^([" + scheme_chars + "]+:)?//")
 
 
-def looks_like_ip(maybe_ip):
+def looks_like_ip(maybe_ip: str) -> bool:
     """Does the given str look like an IP address?"""
     if not maybe_ip[0].isdigit():
         return False
diff --git a/tldextract/suffix_list.py b/tldextract/suffix_list.py
index fba5218..e30efe9 100644
--- a/tldextract/suffix_list.py
+++ b/tldextract/suffix_list.py
@@ -3,10 +3,13 @@
 import logging
 import pkgutil
 import re
+from typing import List, Sequence, Tuple, Union, cast
 
 import requests
 from requests_file import FileAdapter  # type: ignore[import]
 
+from .cache import DiskCache
+
 LOG = logging.getLogger("tldextract")
 
 PUBLIC_SUFFIX_RE = re.compile(r"^(?P<suffix>[.*!]*\w[\S]*)", re.UNICODE | re.MULTILINE)
@@ -18,7 +21,11 @@ class SuffixListNotFound(LookupError):
     you can specify backups, or use this library's bundled snapshot."""
 
 
-def find_first_response(cache, urls, cache_fetch_timeout=None):
+def find_first_response(
+    cache: DiskCache,
+    urls: Sequence[str],
+    cache_fetch_timeout: Union[float, int, None] = None,
+) -> str:
     """Decode the first successfully fetched URL, from UTF-8 encoding to
     Python unicode.
     """
@@ -38,7 +45,7 @@ def find_first_response(cache, urls, cache_fetch_timeout=None):
     )
 
 
-def extract_tlds_from_suffix_list(suffix_list_text):
+def extract_tlds_from_suffix_list(suffix_list_text: str) -> Tuple[List[str], List[str]]:
     """Parse the raw suffix list text for its different designations of
     suffixes."""
     public_text, _, private_text = suffix_list_text.partition(
@@ -50,7 +57,12 @@ def extract_tlds_from_suffix_list(suffix_list_text):
     return public_tlds, private_tlds
 
 
-def get_suffix_lists(cache, urls, cache_fetch_timeout, fallback_to_snapshot):
+def get_suffix_lists(
+    cache: DiskCache,
+    urls: Sequence[str],
+    cache_fetch_timeout: Union[float, int, None],
+    fallback_to_snapshot: bool,
+) -> Tuple[List[str], List[str]]:
     """Fetch, parse, and cache the suffix lists"""
     return cache.run_and_cache(
         func=_get_suffix_lists,
@@ -65,16 +77,22 @@ def get_suffix_lists(cache, urls, cache_fetch_timeout, fallback_to_snapshot):
     )
 
 
-def _get_suffix_lists(cache, urls, cache_fetch_timeout, fallback_to_snapshot):
+def _get_suffix_lists(
+    cache: DiskCache,
+    urls: Sequence[str],
+    cache_fetch_timeout: Union[float, int, None],
+    fallback_to_snapshot: bool,
+) -> Tuple[List[str], List[str]]:
     """Fetch, parse, and cache the suffix lists"""
 
     try:
         text = find_first_response(cache, urls, cache_fetch_timeout=cache_fetch_timeout)
     except SuffixListNotFound as exc:
         if fallback_to_snapshot:
-            text = pkgutil.get_data("tldextract", ".tld_set_snapshot")
-            if not isinstance(text, str):
-                text = str(text, "utf-8")
+            maybe_pkg_data = pkgutil.get_data("tldextract", ".tld_set_snapshot")
+            # package maintainers guarantee file is included
+            pkg_data = cast(bytes, maybe_pkg_data)
+            text = pkg_data.decode("utf-8")
         else:
             raise exc
 
diff --git a/tldextract/tldextract.py b/tldextract/tldextract.py
index b0cec61..f335107 100644
--- a/tldextract/tldextract.py
+++ b/tldextract/tldextract.py
@@ -50,6 +50,7 @@ or suffix were found:
 
 import logging
 import os
+import re
 from functools import wraps
 from typing import FrozenSet, List, NamedTuple, Optional, Sequence, Union
 
@@ -69,6 +70,8 @@ PUBLIC_SUFFIX_LIST_URLS = (
     "https://raw.githubusercontent.com/publicsuffix/list/master/public_suffix_list.dat",
 )
 
+_UNICODE_DOTS_RE = re.compile("[\u002e\u3002\uff0e\uff61]")
+
 
 class ExtractResult(NamedTuple):
     """namedtuple of a URL's subdomain, domain, and suffix."""
@@ -227,7 +230,7 @@ class TLDExtract:
             .rstrip(".")
         )
 
-        labels = netloc.split(".")
+        labels = _UNICODE_DOTS_RE.split(netloc)
 
         translations = [_decode_punycode(label) for label in labels]
         suffix_index = self._get_tld_extractor().suffix_index(
@@ -301,7 +304,8 @@ def extract(  # pylint: disable=missing-function-docstring
 
 
 @wraps(TLD_EXTRACTOR.update)
-def update(*args, **kwargs):  # pylint: disable=missing-function-docstring
+# pylint: disable-next=missing-function-docstring
+def update(*args, **kwargs):  # type: ignore[no-untyped-def]
     return TLD_EXTRACTOR.update(*args, **kwargs)
 
 


Test patch:
diff --git a/tests/integration_test.py b/tests/integration_test.py
index d95f26a..432d7b2 100644
--- a/tests/integration_test.py
+++ b/tests/integration_test.py
@@ -8,5 +8,5 @@ import tldextract
 def test_bad_kwargs():
     with pytest.raises(ValueError):
         tldextract.TLDExtract(
-            cache_dir=False, suffix_list_urls=False, fallback_to_snapshot=False
+            cache_dir=None, suffix_list_urls=(), fallback_to_snapshot=False
         )
diff --git a/tests/main_test.py b/tests/main_test.py
index 6b32211..38066bd 100644
--- a/tests/main_test.py
+++ b/tests/main_test.py
@@ -3,6 +3,7 @@
 import logging
 import os
 import tempfile
+from typing import Sequence, Tuple
 
 import pytest
 import responses
@@ -23,17 +24,17 @@ extract_using_fallback_to_snapshot_no_cache = tldextract.TLDExtract(
 
 
 def assert_extract(
-    url,
-    expected_domain_data,
-    expected_ip_data="",
-    funs=(
+    url: str,
+    expected_domain_data: Tuple[str, str, str, str],
+    expected_ip_data: str = "",
+    funs: Sequence[tldextract.TLDExtract] = (
         extract,
         extract_no_cache,
         extract_using_real_local_suffix_list,
         extract_using_real_local_suffix_list_no_cache,
         extract_using_fallback_to_snapshot_no_cache,
     ),
-):
+) -> None:
     """Test helper to compare all the expected and actual attributes and
     properties of an extraction. Runs the same comparison across several
     permutations of tldextract instance configurations."""
@@ -192,6 +193,10 @@ def test_idna_2008():
         "xn--gieen46ers-73a.de",
         ("xn--gieen46ers-73a.de", "", "xn--gieen46ers-73a", "de"),
     )
+    assert_extract(
+        "angelinablog。com.de",
+        ("angelinablog.com.de", "angelinablog", "com", "de"),
+    )
 
 
 def test_empty():
@@ -346,10 +351,10 @@ def test_cache_timeouts(tmpdir):
 
 def test_tlds_property():
     extract_private = tldextract.TLDExtract(
-        cache_dir=None, suffix_list_urls=None, include_psl_private_domains=True
+        cache_dir=None, suffix_list_urls=(), include_psl_private_domains=True
     )
     extract_public = tldextract.TLDExtract(
-        cache_dir=None, suffix_list_urls=None, include_psl_private_domains=False
+        cache_dir=None, suffix_list_urls=(), include_psl_private_domains=False
     )
     assert len(extract_private.tlds) > len(extract_public.tlds)
 
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 78883e2..3ae30d3 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -2,11 +2,12 @@
 import os.path
 import sys
 import types
+from typing import Any, Dict, Hashable, cast
+from unittest.mock import Mock
 
 import pytest
 import tldextract.cache
-from tldextract.cache import (DiskCache, get_cache_dir,
-                              get_pkg_unique_identifier)
+from tldextract.cache import DiskCache, get_cache_dir, get_pkg_unique_identifier
 
 
 def test_disk_cache(tmpdir):
@@ -28,7 +29,7 @@ def test_get_pkg_unique_identifier(monkeypatch):
     monkeypatch.setattr(sys, "prefix", "/home/john/.pyenv/versions/myvirtualenv")
 
     mock_version_module = types.ModuleType("tldextract._version", "mocked module")
-    mock_version_module.version = "1.2.3"
+    cast(Any, mock_version_module).version = "1.2.3"
     monkeypatch.setitem(sys.modules, "tldextract._version", mock_version_module)
 
     assert (
@@ -72,3 +73,29 @@ def test_get_cache_dir(monkeypatch):
     monkeypatch.setenv("TLDEXTRACT_CACHE", "/alt-tld-cache")
 
     assert get_cache_dir() == "/alt-tld-cache"
+
+
+def test_run_and_cache(tmpdir):
+    cache = DiskCache(tmpdir)
+
+    return_value1 = "unique return value"
+    some_fn = Mock(return_value=return_value1)
+    kwargs1: Dict[str, Hashable] = {"value": 1}
+
+    assert some_fn.call_count == 0
+
+    call1 = cache.run_and_cache(some_fn, "test_namespace", kwargs1, kwargs1.keys())
+    assert call1 == return_value1
+    assert some_fn.call_count == 1
+
+    call2 = cache.run_and_cache(some_fn, "test_namespace", kwargs1, kwargs1.keys())
+    assert call2 == return_value1
+    assert some_fn.call_count == 1
+
+    kwargs2: Dict[str, Hashable] = {"value": 2}
+    return_value2 = "another return value"
+    some_fn.return_value = return_value2
+
+    call3 = cache.run_and_cache(some_fn, "test_namespace", kwargs2, kwargs2.keys())
+    assert call3 == return_value2
+    assert some_fn.call_count == 2