Skip to content

Commit afd6057

Browse files
derek73claude
andcommitted
fix: harden cache invalidation, pickle restore, and test coverage
- SetManager.remove() only fires _on_change when an element was actually removed, avoiding unnecessary cache churn on no-op calls - _CachedUnionMember.__set__ raises TypeError for non-SetManager values instead of silently skipping hook wiring (which would break invalidation) - Constants.__setstate__ guards against malformed/truncated pickle state using _CachedUnionMember introspection, matching __getstate__'s approach - Add comment to __init__ explaining the descriptor-assignment ordering dependency that establishes _pst - UnicodeDecodeError fallback in base.py now preserves actual/expected in the assertion message - Replace duplicate add-title test with pickle round-trip callback test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9abb861 commit afd6057

3 files changed

Lines changed: 30 additions & 9 deletions

File tree

nameparser/config/__init__.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,12 @@ def remove(self, *strings: str) -> Self:
117117
Remove the lower case and no-period version of the string arguments from the set.
118118
Returns ``self`` for chaining.
119119
"""
120+
changed = False
120121
for s in strings:
121122
if (lower := lc(s)) in self.elements:
122123
self.elements.remove(lower)
123-
if self._on_change:
124+
changed = True
125+
if changed and self._on_change:
124126
self._on_change()
125127
return self
126128

@@ -200,11 +202,15 @@ def __get__(self, obj: 'Constants | None', objtype: type | None = None) -> 'SetM
200202
return getattr(obj, self._attr)
201203

202204
def __set__(self, obj: 'Constants', value: SetManager) -> None:
205+
if not isinstance(value, SetManager):
206+
raise TypeError(
207+
f"Expected a SetManager instance, got {type(value).__name__!r}. "
208+
"Wrap your iterable: SetManager(['mr', 'ms'])"
209+
)
203210
previous = getattr(obj, self._attr, None)
204211
if isinstance(previous, SetManager):
205212
previous._on_change = None # detach the replaced manager so it no longer invalidates
206-
if isinstance(value, SetManager):
207-
value._on_change = obj._invalidate_pst
213+
value._on_change = obj._invalidate_pst
208214
obj._invalidate_pst()
209215
setattr(obj, self._attr, value)
210216

@@ -316,6 +322,9 @@ def __init__(self,
316322
capitalization_exceptions: TupleManager[str] | Iterable[tuple[str, str]] = CAPITALIZATION_EXCEPTIONS,
317323
regexes: RegexTupleManager | TupleManager[re.Pattern[str]] | Iterable[tuple[str, re.Pattern[str]]] = REGEXES
318324
) -> None:
325+
# These four descriptor assignments call _CachedUnionMember.__set__, which
326+
# calls _invalidate_pst() and establishes self._pst. They must come before
327+
# any read of suffixes_prefixes_titles.
319328
self.prefixes = SetManager(prefixes)
320329
self.suffix_acronyms = SetManager(suffix_acronyms)
321330
self.suffix_not_acronyms = SetManager(suffix_not_acronyms)
@@ -355,6 +364,16 @@ def __setstate__(self, state: Mapping[str, Any]) -> None:
355364
if isinstance(getattr(type(self), name, None), property):
356365
continue
357366
setattr(self, name, value)
367+
# Verify each descriptor-backed attr was restored. Without this, a missing
368+
# key surfaces later as AttributeError: 'Constants' object has no attribute
369+
# '_prefixes' — the private mangled name, not the public one, making it
370+
# very hard to diagnose.
371+
for attr in (n for n, v in vars(type(self)).items() if isinstance(v, _CachedUnionMember)):
372+
if not hasattr(self, '_' + attr):
373+
raise ValueError(
374+
f"Pickle state is missing required field {attr!r}. "
375+
"The state blob may be truncated or from an incompatible version."
376+
)
358377

359378
def __getstate__(self) -> Mapping[str, Any]:
360379
# Pickle the instance's own configuration: the collections built in

tests/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def m(self, actual: T, expected: T, hn: HumanName) -> None:
2525
hn,
2626
)
2727
except UnicodeDecodeError:
28-
assert actual == expected_
28+
assert actual == expected_, f"actual={actual!r} != expected={expected_!r}"
2929

3030
def assertEqual(self, first: object, second: object, msg: object = None) -> None:
3131
assert first == second, msg

tests/test_constants.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,14 @@ def test_suffixes_prefixes_titles_reflects_add_suffix_not_acronym(self) -> None:
277277
c.suffix_not_acronyms.add('xsfx')
278278
self.assertIn('xsfx', c.suffixes_prefixes_titles)
279279

280-
def test_suffixes_prefixes_titles_reflects_add_after_initial_read(self) -> None:
281-
"""suffixes_prefixes_titles must reflect mutations even after the cache has been primed."""
280+
def test_pickle_roundtrip_rewires_invalidation_callbacks(self) -> None:
281+
"""Mutations on a deserialized Constants must still invalidate the cache."""
282282
c = Constants()
283-
_ = c.suffixes_prefixes_titles # prime the cache
284-
c.titles.add('emerita')
285-
self.assertIn('emerita', c.suffixes_prefixes_titles)
283+
# Safe: round-tripping a Constants the test just built, not untrusted data.
284+
restored = pickle.loads(pickle.dumps(c))
285+
_ = restored.suffixes_prefixes_titles # prime the cache
286+
restored.titles.add('posttitle')
287+
self.assertIn('posttitle', restored.suffixes_prefixes_titles)
286288

287289
def test_is_rootname_consistent_with_is_title(self) -> None:
288290
"""is_rootname must return False for words recognised by is_title."""

0 commit comments

Comments
 (0)