Skip to content

Commit 6b50956

Browse files
developer0hyeclaude
andcommitted
fix(analyzer): make every listed recognizer loadable from the registry YAML
Three recognizers ship in default_recognizers.yaml with enabled: false but cannot be turned on. KrBrnRecognizer, KrDriverLicenseRecognizer and UsMbiRecognizer do not accept the name keyword argument that RecognizerListLoader passes to every predefined recognizer, so flipping enabled to true raises TypeError: __init__() got an unexpected keyword argument 'name' before the registry finishes loading. enabled: false is an opt-in switch, not a disclaimer: an entry that cannot be enabled should not be listed. The failure also reads as a user configuration error even though the YAML is correct, unlike the optional-dependency entries, which refuse to load with an actionable ImportError. Adds the argument to the three constructors, and adds contract tests so the next one is caught by CI. Why this survived: the constructor signature is part of a contract that nothing enforced. Each recognizer's own tests instantiate the class directly, where no name is passed, so they all pass. The registry-level tests build the default configuration, in which roughly 60 entries are disabled and therefore never constructed. Nothing in between ever looked. The new tests close that gap from both sides: - Every predefined PatternRecognizer subclass must accept the kwargs the loader passes. This fires when the class is added, before it reaches the YAML at all, which is the point at which KrPassportRecognizer went wrong in #1814. - Every entry in default_recognizers.yaml must resolve to a class and must load once enabled, exercised entry by entry so a failure names the recognizer. Optional-dependency recognizers are skipped. - A class_name plus name entry must produce an instance with the configured name, which is the documented reason the loader passes name and what makes the kwarg contract load-bearing. Verified by reverting the three constructors: the signature test and the load test each fail for exactly those three, with no other failures. Non-pattern recognizers are out of scope. Several are not registrable from default_recognizers.yaml, and AzureAILanguageRecognizer deliberately fixes its own display name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent eae9403 commit 6b50956

4 files changed

Lines changed: 179 additions & 0 deletions

File tree

presidio-analyzer/presidio_analyzer/predefined_recognizers/country_specific/korea/kr_brn_recognizer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ def __init__(
5858
supported_language: str = "ko",
5959
supported_entity: str = "KR_BRN",
6060
replacement_pairs: Optional[List[Tuple[str, str]]] = None,
61+
name: Optional[str] = None,
6162
):
6263
self.replacement_pairs = replacement_pairs if replacement_pairs else [("-", "")]
6364

@@ -68,6 +69,7 @@ def __init__(
6869
patterns=patterns,
6970
context=context,
7071
supported_language=supported_language,
72+
name=name,
7173
)
7274

7375
def validate_result(self, pattern_text: str) -> Union[bool, None]:

presidio-analyzer/presidio_analyzer/predefined_recognizers/country_specific/korea/kr_driver_license_recognizer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ def __init__(
7070
supported_language: str = "ko",
7171
supported_entity: str = "KR_DRIVER_LICENSE",
7272
replacement_pairs: Optional[List[Tuple[str, str]]] = None,
73+
name: Optional[str] = None,
7374
):
7475
self.replacement_pairs = (
7576
replacement_pairs if replacement_pairs else [("-", ""), (" ", "")]
@@ -82,6 +83,7 @@ def __init__(
8283
patterns=patterns,
8384
context=context,
8485
supported_language=supported_language,
86+
name=name,
8587
)
8688

8789
def validate_result(self, pattern_text: str) -> bool:

presidio-analyzer/presidio_analyzer/predefined_recognizers/country_specific/us/us_mbi_recognizer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ def __init__(
9090
context: Optional[List[str]] = None,
9191
supported_language: str = "en",
9292
supported_entity: str = "US_MBI",
93+
name: Optional[str] = None,
9394
):
9495
patterns = patterns if patterns else self.PATTERNS
9596
context = context if context else self.CONTEXT
@@ -98,4 +99,5 @@ def __init__(
9899
patterns=patterns,
99100
context=context,
100101
supported_language=supported_language,
102+
name=name,
101103
)
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
"""Contract tests between the registry loader and predefined recognizers.
2+
3+
``RecognizerListLoader`` builds predefined recognizers from YAML by passing the
4+
entry's keys as constructor kwargs. That makes the constructor signature part of
5+
a contract which nothing else enforces: a recognizer can satisfy every one of
6+
its own unit tests -- which instantiate it directly -- and still be impossible
7+
to load from a registry configuration.
8+
9+
The gap is specifically in the *disabled* entries. ``default_recognizers.yaml``
10+
ships ~60 recognizers with ``enabled: false``, and no other test constructs
11+
them, so a broken constructor stays invisible until a user flips the switch.
12+
These tests construct every one of them.
13+
"""
14+
15+
import inspect
16+
from pathlib import Path
17+
from typing import Dict, List
18+
19+
import presidio_analyzer.predefined_recognizers as predefined
20+
import pytest
21+
import yaml
22+
from presidio_analyzer import EntityRecognizer, PatternRecognizer
23+
from presidio_analyzer.recognizer_registry import RecognizerRegistryProvider
24+
from presidio_analyzer.recognizer_registry.recognizers_loader_utils import (
25+
RecognizerListLoader,
26+
)
27+
28+
DEFAULT_CONF = (
29+
Path(__file__).resolve().parent.parent
30+
/ "presidio_analyzer"
31+
/ "conf"
32+
/ "default_recognizers.yaml"
33+
)
34+
35+
# Kwargs ``RecognizerListLoader`` passes to every predefined recognizer it
36+
# builds: ``name`` comes from the YAML entry (or its ``class_name`` alias) and
37+
# ``supported_language`` from the resolved per-language configuration.
38+
LOADER_KWARGS = ("name", "supported_language")
39+
40+
41+
def _pattern_recognizer_classes() -> Dict[str, type]:
42+
"""Predefined ``PatternRecognizer`` subclasses, which the YAML loader builds.
43+
44+
Non-pattern recognizers (NER/LLM/remote wrappers) are excluded: several are
45+
not registrable from ``default_recognizers.yaml`` and some deliberately fix
46+
their own display name.
47+
"""
48+
classes = {}
49+
for attr in dir(predefined):
50+
obj = getattr(predefined, attr)
51+
if not isinstance(obj, type) or not issubclass(obj, EntityRecognizer):
52+
continue
53+
if obj in (EntityRecognizer, PatternRecognizer):
54+
continue
55+
if issubclass(obj, PatternRecognizer):
56+
classes[attr] = obj
57+
return classes
58+
59+
60+
def _yaml_entries() -> List[Dict]:
61+
"""Normalize the shipped recognizer list to dict entries."""
62+
data = yaml.safe_load(DEFAULT_CONF.read_text(encoding="utf-8"))
63+
entries = []
64+
for entry in data["recognizers"]:
65+
entries.append({"name": entry} if isinstance(entry, str) else dict(entry))
66+
return entries
67+
68+
69+
def _entry_languages(entry: Dict) -> List[str]:
70+
"""Languages an entry declares, in either supported YAML shape."""
71+
languages = entry.get("supported_languages")
72+
if not languages:
73+
return ["en"]
74+
if isinstance(languages[0], str):
75+
return list(languages)
76+
return [item["language"] for item in languages]
77+
78+
79+
def _entry_id(entry: Dict) -> str:
80+
return entry.get("class_name") or entry["name"]
81+
82+
83+
PATTERN_CLASSES = _pattern_recognizer_classes()
84+
YAML_ENTRIES = _yaml_entries()
85+
86+
87+
def test_default_conf_has_entries():
88+
"""Guard the fixtures themselves: an empty parse would pass everything."""
89+
assert PATTERN_CLASSES, "no predefined PatternRecognizer subclasses found"
90+
assert YAML_ENTRIES, "no recognizers parsed from default_recognizers.yaml"
91+
92+
93+
@pytest.mark.parametrize("class_name", sorted(PATTERN_CLASSES))
94+
def test_pattern_recognizer_accepts_loader_kwargs(class_name):
95+
"""Constructor must accept every kwarg the YAML loader passes.
96+
97+
Catches the defect before the recognizer reaches the YAML at all: a class
98+
added without ``name`` passes its own unit tests, and only fails once
99+
someone tries to register it.
100+
"""
101+
parameters = inspect.signature(PATTERN_CLASSES[class_name].__init__).parameters
102+
if any(p.kind == inspect.Parameter.VAR_KEYWORD for p in parameters.values()):
103+
return
104+
missing = [kwarg for kwarg in LOADER_KWARGS if kwarg not in parameters]
105+
assert not missing, (
106+
f"{class_name}.__init__ does not accept {missing}, which "
107+
f"RecognizerListLoader passes to every predefined recognizer. "
108+
f"Loading it from a registry YAML raises TypeError."
109+
)
110+
111+
112+
@pytest.mark.parametrize("entry", YAML_ENTRIES, ids=_entry_id)
113+
def test_yaml_entry_class_resolves(entry):
114+
"""Every shipped entry must name a real recognizer class."""
115+
RecognizerListLoader.get_existing_recognizer_cls(recognizer_name=_entry_id(entry))
116+
117+
118+
@pytest.mark.parametrize("entry", YAML_ENTRIES, ids=_entry_id)
119+
def test_yaml_entry_loads_when_enabled(entry):
120+
"""Every shipped entry must load once ``enabled`` is true.
121+
122+
``enabled: false`` is an opt-in switch, not a disclaimer -- an entry that
123+
cannot be turned on should not be listed. Recognizers gated behind an
124+
optional dependency are skipped: refusing to load with an actionable
125+
ImportError is their intended behavior.
126+
"""
127+
entry = dict(entry, enabled=True)
128+
configuration = {
129+
"global_regex_flags": yaml.safe_load(DEFAULT_CONF.read_text(encoding="utf-8"))[
130+
"global_regex_flags"
131+
],
132+
"supported_languages": _entry_languages(entry),
133+
"recognizers": [entry],
134+
}
135+
136+
try:
137+
registry = RecognizerRegistryProvider(
138+
registry_configuration=configuration
139+
).create_recognizer_registry()
140+
except ImportError as exc:
141+
pytest.skip(f"{_entry_id(entry)} needs an optional dependency: {exc}")
142+
143+
assert registry.recognizers, (
144+
f"{_entry_id(entry)} is listed in default_recognizers.yaml but loaded "
145+
f"nothing for languages {_entry_languages(entry)}"
146+
)
147+
148+
149+
def test_yaml_entry_can_be_renamed_via_class_name():
150+
"""``class_name`` + ``name`` must give the instance the configured name.
151+
152+
This is the documented reason the loader passes ``name`` at all (see
153+
``RecognizerListLoader.get_recognizer_name``), so it is the behavior that
154+
makes the kwarg contract above load-bearing rather than incidental.
155+
"""
156+
configuration = {
157+
"global_regex_flags": 26,
158+
"supported_languages": ["en"],
159+
"recognizers": [
160+
{
161+
"class_name": "UsSsnRecognizer",
162+
"name": "MyRenamedSsnRecognizer",
163+
"supported_languages": ["en"],
164+
"type": "predefined",
165+
"country_code": "us",
166+
}
167+
],
168+
}
169+
registry = RecognizerRegistryProvider(
170+
registry_configuration=configuration
171+
).create_recognizer_registry()
172+
173+
assert [r.name for r in registry.recognizers] == ["MyRenamedSsnRecognizer"]

0 commit comments

Comments
 (0)