From a574f56df5a449e07416e1651f272e22f3cdf0cf Mon Sep 17 00:00:00 2001 From: Libby Date: Wed, 11 Nov 2026 06:45:40 +0000 Subject: [PATCH] cultcache-py: a batch naming a key twice lands the last record; refusal text in C# order; alias-in-both judged in DocumentDefinition; load freshness pinned Co-Authored-By: Claude Sonnet 5.5 --- .../cultcache-py/src/cultcache_py/cache.py | 57 +++++---- .../src/cultcache_py/documents.py | 17 ++- .../cultcache-py/tests/test_unique_index.py | 121 ++++++++++++++++-- 3 files changed, 153 insertions(+), 42 deletions(-) diff --git a/packages/cultcache-py/src/cultcache_py/cache.py b/packages/cultcache-py/src/cultcache_py/cache.py index f094e42c..c934c33b 100644 --- a/packages/cultcache-py/src/cultcache_py/cache.py +++ b/packages/cultcache-py/src/cultcache_py/cache.py @@ -74,7 +74,12 @@ _BLANK_INDEX_CHARS = frozenset( ) -def _index_text(value: Any) -> str | None: +def _utf16_order(text: str) -> bytes: + """Sorts as C# StringComparer.Ordinal does: by UTF-16 code unit, not code point.""" + return text.encode("utf-16-be", "surrogatepass") + + +def index_text(value: Any) -> str | None: """The canonical text of an index value, as C# IndexValue and RenderCanonicalNumber, or None when it names no value: a string is as is (never trimmed), a boolean is `True`/`False`, an integer is its digits, a non-integral finite float is its exact decimal expansion, NaN and infinities and blank strings are no value.""" @@ -475,6 +480,9 @@ class CultCache: """Validates every record (home and name and index extractors included) before the store is touched; once the store accepts, applying to the cache has no fallible step.""" type = document.type + # A batch naming a key twice lands the last record for it, as C# CultCacheBatch stages one operation per key. The + # judge, the store and the apply all read this one list. + records = list({envelope.key: (envelope, value) for envelope, value in records}.values()) name_extractor = self._state.name_extractors.get(type) index_extractors = self._state.index_extractors.get(type, {}) admitted = [ @@ -482,10 +490,9 @@ class CultCache: for envelope, value in records ] self._refuse_index_sharing( - type, - {envelope.key: keys for envelope, _, keys in admitted}, + [(type, envelope.key, keys) for envelope, _, keys in admitted], self._state.indexes, - [envelope.key for envelope, _ in records], + {(type, envelope.key) for envelope, _ in records}, ) store = self._store_for_type(type) legacy_key = self._state.legacy_global_keys.get(type) @@ -601,7 +608,7 @@ class CultCache: name = None if name_extractor is None else extract_value(value, name_extractor) indexes: dict[str, str] = {} for index, extractor in index_extractors.items(): - index_value = _index_text(extract_value(value, extractor)) + index_value = index_text(extract_value(value, extractor)) if index_value is not None: indexes[index] = index_value return _LookupKeys(None if name is None else str(name), indexes) @@ -621,8 +628,12 @@ class CultCache: type_index_extractors = index_extractors.get(type, {}) for key, value in values.items(): lookup_keys.setdefault(type, {})[key] = self._lookup_keys_for(value, name_extractor, type_index_extractors) - # Judged whole over every alias: a held value can have been mutated in place since it was admitted. - self._refuse_index_sharing(type, lookup_keys.get(type, {}), {}, ()) + # Judged whole, over every type at once and on the file alone (never against what the cache held): a held value can + # have been mutated in place since it was admitted, and another process may have moved a value between records. + self._refuse_index_sharing( + [(type, key, keys) for type, by_key in lookup_keys.items() for key, keys in by_key.items()], {}, () + ) + for type in lookup_keys: for key, keys in lookup_keys.get(type, {}).items(): if keys.name is not None: names[(type, keys.name)] = key @@ -632,30 +643,30 @@ class CultCache: def _refuse_index_sharing( self, - type: str, - landing: dict[str, _LookupKeys], + landing: list[tuple[str, str, _LookupKeys]], held: dict[tuple[str, str, str], str], - replaced: Iterable[str], + replaced: Iterable[tuple[str, str]], ) -> None: - """The one index rule (variants:ruling:unique-indexes): within one type and one unique alias, no two records - hold one non-blank value, judged on the result. The holders of a value are the landing keys projecting it plus - the held key unless this admission replaces it; two distinct holders refuse, naming the two smallest keys - (ordinal) and the alias, never the value. Aliases are judged in ordinal order, values in ordinal order.""" + """The one index rule (variants:ruling:unique-indexes), in the order of C# RefuseIndexSharing: within one type and one + unique alias, no two records hold one non-blank value, judged on the result. The holders of a value are the landing + keys projecting it (in landing order) plus the held key unless this admission replaces it; two distinct holders + refuse, naming the two smallest keys (UTF-16 code-unit order) and the alias, never the value. Aliases are judged in + UTF-16 order, values in the order they landed.""" released = set(replaced) - holders: dict[tuple[str, str], set[str]] = {} - for key, keys in landing.items(): + landed: dict[tuple[str, str, str], list[str]] = {} + for type, key, keys in landing: for index, index_value in keys.indexes.items(): - holders.setdefault((index, index_value), set()).add(key) - for (index, index_value), keys in sorted(holders.items()): + landed.setdefault((type, index, index_value), []).append(key) + for (type, index, index_value), keys in sorted(landed.items(), key=lambda item: _utf16_order(item[0][1])): held_key = held.get((type, index, index_value)) - if held_key is not None and held_key not in released: - keys = keys | {held_key} - if len(keys) > 1: - first, second = sorted(keys)[:2] + if held_key is not None and (type, held_key) not in released: + keys = [*keys, held_key] + distinct = sorted(dict.fromkeys(keys), key=_utf16_order) + if len(distinct) > 1: document = self._state.documents.get(type) schema_name = (document.schema_name if document is not None else None) or type raise CultCacheError( - f"Records {first} and {second} ({schema_name}) both hold unique index '{index}'; " + f"Records {distinct[0]} and {distinct[1]} ({schema_name}) both hold unique index '{index}'; " "an index value names one record." ) diff --git a/packages/cultcache-py/src/cultcache_py/documents.py b/packages/cultcache-py/src/cultcache_py/documents.py index d0cdde3a..6101c136 100644 --- a/packages/cultcache-py/src/cultcache_py/documents.py +++ b/packages/cultcache-py/src/cultcache_py/documents.py @@ -195,6 +195,14 @@ class DocumentDefinition(Generic[T]): self.validate(value) return value + def __post_init__(self) -> None: + # Judged here so every constructor passes it: the factories, the dataclass constructor and dataclasses.replace. + for alias in self.roles: + if alias in self.indexes: + raise ValueError( + f"Document type {self.type} declares '{alias}' as both an index and a role; an index already names its role." + ) + def catalog_entry(self) -> CultCacheSchemaCatalogEntry: if self._catalog_entry is not None: return self._catalog_entry @@ -237,14 +245,6 @@ class DocumentDefinition(Generic[T]): return catalog_entry -def _refuse_shared_aliases(type: str, indexes: dict[str, Any] | None, roles: dict[str, Any] | None) -> None: - for alias in roles or (): - if alias in (indexes or {}): - raise ValueError( - f"Document type {type} declares '{alias}' as both an index and a role; an index already names its role." - ) - - def define_document_type( type: str, *, @@ -265,7 +265,6 @@ def define_document_type( compatible_schema_ids: Iterable[str] | None = None, members: Iterable[CultCacheSchemaCatalogMember] | Callable[[], tuple[CultCacheSchemaCatalogMember, ...]] | None = None, ) -> DocumentDefinition[T]: - _refuse_shared_aliases(type, indexes, roles) return DocumentDefinition( type=type, encode=encode, diff --git a/packages/cultcache-py/tests/test_unique_index.py b/packages/cultcache-py/tests/test_unique_index.py index 91ea33a8..0dca2aa8 100644 --- a/packages/cultcache-py/tests/test_unique_index.py +++ b/packages/cultcache-py/tests/test_unique_index.py @@ -19,7 +19,7 @@ from cultcache_py import ( leaves as t, database_entry_field, ) -from cultcache_py.cache import CultCacheError, _index_text +from cultcache_py.cache import CultCacheError, index_text # variants:ruling:unique-indexes. `indexes` are unique within one type, `roles` are selection aliases. The vectors in # tests/vectors/unique-index are written by the C# reference; the rest of the behaviour is pinned here. @@ -108,36 +108,36 @@ class IndexTextTests(unittest.TestCase): ] for value, text in cases: with self.subTest(value=value): - self.assertEqual(_index_text(value), text) - self.assertIsInstance(_index_text(value), str) + self.assertEqual(index_text(value), text) + self.assertIsInstance(index_text(value), str) def test_no_value(self) -> None: blank = ["", " ", "\t", "\n", "\x0b", "\x0c", "\r", "\x85", "\xa0", " ", "
", "
", " ", " ", "　", " 　\x85 ", *map(chr, range(0x2000, 0x200B))] for value in [None, float("nan"), float("inf"), float("-inf"), *blank]: with self.subTest(value=repr(value)): - self.assertIsNone(_index_text(value)) + self.assertIsNone(index_text(value)) def test_blank_set_is_exactly_the_csharp_set(self) -> None: csharp = {*range(0x09, 0x0E), 0x20, 0x85, 0xA0, 0x1680, *range(0x2000, 0x200B), 0x2028, 0x2029, 0x202F, 0x205F, 0x3000} self.assertEqual(len(csharp), 25) - got = {code for code in range(0x110000) if 0xD800 > code or code > 0xDFFF if _index_text(chr(code)) is None} + got = {code for code in range(0x110000) if 0xD800 > code or code > 0xDFFF if index_text(chr(code)) is None} self.assertEqual(got, csharp) # Python's isspace is the wrong test: it also counts U+001C-001F. self.assertTrue("\x1f".isspace()) - self.assertEqual(_index_text("\x1f"), "\x1f") + self.assertEqual(index_text("\x1f"), "\x1f") def test_floats_render_exactly_not_through_str(self) -> None: for value in (0.1, 1e-7, 123456789.123456789, 5e-324, 1.7976931348623157e308): with self.subTest(value=value): - text = _index_text(value) + text = index_text(value) self.assertNotIn("e", text.lower()) self.assertEqual(Decimal(text), Decimal(value)) self.assertFalse(text.endswith("0") and "." in text) def test_bool_is_not_an_integer(self) -> None: - self.assertEqual(_index_text(True), "True") - self.assertNotEqual(_index_text(True), "1") + self.assertEqual(index_text(True), "True") + self.assertNotEqual(index_text(True), "1") class UniqueIndexTests(unittest.TestCase): @@ -425,7 +425,7 @@ class VectorTests(unittest.TestCase): document = item_doc(indexes={"sku": "sku"}, roles={}) for label, kind, value, text in msgpack.unpackb((VECTORS / "values.msgpack").read_bytes(), raw=False): with self.subTest(label=label): - self.assertEqual(_index_text(value), text) + self.assertEqual(index_text(value), text) cache = cache_of(document) cache.put(document, "item:1", Item(sku=value)) if text is None: @@ -470,5 +470,106 @@ class VectorTests(unittest.TestCase): self.assertEqual(path.read_bytes(), before) +class ListStore(RecordingStore): + """A store whose contents the test swaps between loads: another process rewrote the file.""" + + def __init__(self, envelopes=()) -> None: + super().__init__() + self.envelopes = list(envelopes) + + def pull_all(self): + return list(self.envelopes) + + +class BatchAndOrderTests(unittest.TestCase): + def test_a_batch_naming_a_key_twice_lands_the_last_record_and_keeps_every_held_lookup(self) -> None: + # C# CultCacheBatch stages one operation per key, the last one named. + document = item_doc() + store = RecordingStore() + cache = cache_of(document, store=store) + cache.put(document, "item:h", Item(sku="S")) + store.pushed.clear() + cache.put_envelopes( + document, [envelope(document, "item:k", Item(sku="S")), envelope(document, "item:k", Item(sku="T"))] + ) + self.assertEqual(store.pushed, ["item:k"]) + self.assertEqual(cache.get_required(document, "item:k").sku, "T") + self.assertEqual(cache.get_key_by_index(document, "sku", "S"), "item:h") + self.assertEqual(cache.get_key_by_index(document, "sku", "T"), "item:k") + with self.assertRaises(CultCacheError) as caught: + cache.put(document, "item:z", Item(sku="S")) + self.assertEqual(str(caught.exception), refusal("item:h", "item:z", "item", "sku")) + + def test_a_batch_naming_a_key_twice_is_judged_on_the_record_that_lands(self) -> None: + document = item_doc() + cache = cache_of(document) + cache.put(document, "item:h", Item(sku="S")) + # the first record for k would collide with h; only the last lands, so the batch is admitted + cache.put_envelopes( + document, [envelope(document, "item:k", Item(sku="S")), envelope(document, "item:k", Item(sku="T"))] + ) + # and the reverse order lands the colliding record, which is refused + with self.assertRaises(CultCacheError): + cache.put_envelopes( + document, [envelope(document, "item:j", Item(sku="U")), envelope(document, "item:j", Item(sku="S"))] + ) + self.assertNotIn("item:j", cache.snapshot().get("item", {})) + + def test_values_are_named_in_landing_order(self) -> None: + document = item_doc() + cache = cache_of(document) + rows = [("item:a", "B"), ("item:b", "B"), ("item:x", "A"), ("item:y", "A")] + with self.assertRaises(CultCacheError) as caught: + cache.put_envelopes(document, [envelope(document, key, Item(sku=sku)) for key, sku in rows]) + self.assertEqual(str(caught.exception), refusal("item:a", "item:b", "item", "sku")) + + def test_keys_sort_by_utf16_code_unit_not_code_point(self) -> None: + document = item_doc() + cache = cache_of(document) + astral, bmp = "k\U0001F600", "k\uff01" # code points: bmp < astral; UTF-16 units: D83D < FF01 + with self.assertRaises(CultCacheError) as caught: + cache.put_envelopes(document, [envelope(document, bmp, Item(sku="S")), envelope(document, astral, Item(sku="S"))]) + self.assertEqual(str(caught.exception), refusal(astral, bmp, "item", "sku")) + + def test_a_load_judges_every_type_at_once_aliases_in_ordinal_order(self) -> None: + first = item_doc("first", indexes={"zeta": "sku"}, roles={}) + second = item_doc("second", indexes={"alpha": "sku"}, roles={}) + loose_first, loose_second = item_doc("first", indexes={}, roles={}), item_doc("second", indexes={}, roles={}) + store = ListStore( + [envelope(loose_first, "f:1", Item(sku="S")), envelope(loose_first, "f:2", Item(sku="S"))] + + [envelope(loose_second, "s:1", Item(sku="S")), envelope(loose_second, "s:2", Item(sku="S"))] + ) + cache = cache_of(first, second, store=store) + with self.assertRaises(CultCacheError) as caught: + cache.pull_all_backing_stores() + self.assertEqual(str(caught.exception), refusal("s:1", "s:2", "second", "alpha")) + + +class LoadFreshnessTests(unittest.TestCase): + def test_a_load_is_judged_on_the_file_alone_not_against_the_held_map(self) -> None: + document = item_doc() + store = ListStore([envelope(document, "item:b", Item(sku="S")), envelope(document, "item:c", Item(sku="T"))]) + cache = cache_of(document, store=store) + cache.pull_all_backing_stores() + self.assertEqual(cache.get_key_by_index(document, "sku", "S"), "item:b") + # another process moved S from b to a, and b to T's old place + store.envelopes = [envelope(document, "item:a", Item(sku="S")), envelope(document, "item:b", Item(sku="U")), store.envelopes[1]] + cache.pull_all_backing_stores() + self.assertEqual(cache.get_key_by_index(document, "sku", "S"), "item:a") + self.assertEqual(cache.get_key_by_index(document, "sku", "U"), "item:b") + + +class DefinitionTests(unittest.TestCase): + def test_every_constructor_refuses_an_alias_in_both_indexes_and_roles(self) -> None: + from dataclasses import replace + + from cultcache_py import DocumentDefinition + + with self.assertRaisesRegex(ValueError, "both an index and a role"): + DocumentDefinition(type="item", indexes={"sku": "sku"}, roles={"sku": "sku"}) + with self.assertRaisesRegex(ValueError, "both an index and a role"): + replace(item_doc(roles={}), roles={"sku": "sku"}) + + if __name__ == "__main__": unittest.main()