Skip to content

Commit da42156

Browse files
Fix critical bugs and add missing exception handling
- Fix delete_key: directly call DeleteKey instead of opening key first - Fix delete_sub_keys: enumerate at index 0 (keys shift after deletion) - Add exception handling to set_reg_sz_value and create_key (now return bool) - Fix TTL calculation in memory store (recalculate_ttl before use) - Use else blocks in try-except for TRY300 linting compliance - Update sync package to match async changes Co-authored-by: William Easton <[email protected]>
1 parent 7c15e54 commit da42156

File tree

4 files changed

+139
-33
lines changed
  • key-value
    • key-value-aio/src/key_value/aio/stores
    • key-value-sync/src/key_value/sync/code_gen/stores

4 files changed

+139
-33
lines changed

key-value/key-value-aio/src/key_value/aio/stores/memory/store.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ def put(self, key: str, value: ManagedEntry) -> None:
153153
key: The key to store under.
154154
value: The ManagedEntry to store.
155155
"""
156+
# Recalculate TTL to get remaining time until expiration
157+
value.recalculate_ttl()
156158
self._cache[key] = MemoryCacheEntry.from_managed_entry(managed_entry=value, ttl=value.ttl)
157159

158160
def delete(self, key: str) -> bool:

key-value/key-value-aio/src/key_value/aio/stores/windows_registry/utils.py

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,24 @@ def get_reg_sz_value(hive: HiveType, sub_key: str, value_name: str) -> str | Non
2323
return None
2424

2525

26-
def set_reg_sz_value(hive: HiveType, sub_key: str, value_name: str, value: str) -> None:
26+
def set_reg_sz_value(hive: HiveType, sub_key: str, value_name: str, value: str) -> bool:
2727
"""Set a string value in the Windows Registry.
2828
2929
Args:
3030
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
3131
sub_key: The registry subkey path.
3232
value_name: The name of the registry value to set.
3333
value: The string value to write.
34+
35+
Returns:
36+
True if the value was set, False if the key doesn't exist or couldn't be accessed.
3437
"""
35-
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
36-
winreg.SetValueEx(reg_key, value_name, 0, winreg.REG_SZ, value)
38+
try:
39+
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
40+
winreg.SetValueEx(reg_key, value_name, 0, winreg.REG_SZ, value)
41+
return True
42+
except (FileNotFoundError, OSError):
43+
return False
3744

3845

3946
def delete_reg_sz_value(hive: HiveType, sub_key: str, value_name: str) -> bool:
@@ -72,14 +79,22 @@ def has_key(hive: HiveType, sub_key: str) -> bool:
7279
return False
7380

7481

75-
def create_key(hive: HiveType, sub_key: str) -> None:
82+
def create_key(hive: HiveType, sub_key: str) -> bool:
7683
"""Create a new registry key.
7784
7885
Args:
7986
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
8087
sub_key: The registry subkey path to create.
88+
89+
Returns:
90+
True if the key was created or already exists, False on error.
8191
"""
82-
winreg.CreateKey(hive, sub_key)
92+
try:
93+
winreg.CreateKey(hive, sub_key)
94+
except OSError:
95+
return False
96+
else:
97+
return True
8398

8499

85100
def delete_key(hive: HiveType, sub_key: str) -> bool:
@@ -93,32 +108,34 @@ def delete_key(hive: HiveType, sub_key: str) -> bool:
93108
True if the key was deleted, False if it didn't exist or couldn't be deleted.
94109
"""
95110
try:
96-
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
97-
winreg.DeleteKey(reg_key, sub_key)
98-
return True
111+
winreg.DeleteKey(hive, sub_key)
99112
except (FileNotFoundError, OSError):
100113
return False
114+
else:
115+
return True
101116

102117

103118
def delete_sub_keys(hive: HiveType, sub_key: str) -> None:
104-
"""Delete all subkeys of a registry key.
119+
"""Delete all immediate subkeys of a registry key.
105120
106-
This function recursively deletes all subkeys under the specified registry key.
121+
This function deletes all immediate child keys (non-recursive).
107122
108123
Args:
109124
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
110125
sub_key: The registry subkey path whose subkeys should be deleted.
111126
"""
112127
try:
113128
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE | winreg.KEY_ENUMERATE_SUB_KEYS) as reg_key:
114-
index = 0
115129
while True:
116-
if not (next_child_key := winreg.EnumKey(reg_key, index)):
130+
try:
131+
# Always enumerate at index 0 since keys shift after deletion
132+
next_child_key = winreg.EnumKey(reg_key, 0)
133+
except OSError:
134+
# No more subkeys
117135
break
118136

119-
with contextlib.suppress(Exception):
137+
# Key already deleted or can't be deleted, skip it
138+
with contextlib.suppress(FileNotFoundError, OSError):
120139
winreg.DeleteKey(reg_key, next_child_key)
121-
122-
index += 1
123140
except (FileNotFoundError, OSError):
124141
return

key-value/key-value-sync/src/key_value/sync/code_gen/stores/memory/store.py

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@
2727

2828
@dataclass
2929
class MemoryCacheEntry:
30-
"""A cache entry for the memory store."""
30+
"""A cache entry for the memory store.
31+
32+
This dataclass represents an entry in the MemoryStore cache, storing the JSON-serialized
33+
value along with expiration metadata and the original TTL value at insertion time.
34+
"""
3135

3236
json_str: str
3337

@@ -37,14 +41,40 @@ class MemoryCacheEntry:
3741

3842
@classmethod
3943
def from_managed_entry(cls, managed_entry: ManagedEntry, ttl: SupportsFloat | None = None) -> Self:
44+
"""Create a cache entry from a ManagedEntry.
45+
46+
Args:
47+
managed_entry: The ManagedEntry to convert.
48+
ttl: The TTL value at insertion time.
49+
50+
Returns:
51+
A new MemoryCacheEntry.
52+
"""
4053
return cls(json_str=managed_entry.to_json(), expires_at=managed_entry.expires_at, ttl_at_insert=ttl)
4154

4255
def to_managed_entry(self) -> ManagedEntry:
56+
"""Convert this cache entry to a ManagedEntry.
57+
58+
Returns:
59+
A ManagedEntry reconstructed from the stored JSON.
60+
"""
4361
return ManagedEntry.from_json(json_str=self.json_str)
4462

4563

4664
def _memory_cache_ttu(_key: Any, value: MemoryCacheEntry, now: float) -> float:
47-
"""Calculate time-to-use for cache entries based on their TTL."""
65+
"""Calculate time-to-use for cache entries based on their TTL.
66+
67+
This function is used by TLRUCache to determine when entries should expire.
68+
It calculates the expiration timestamp and updates the entry's expires_at field.
69+
70+
Args:
71+
_key: The cache key (unused).
72+
value: The cache entry.
73+
now: The current time as an epoch timestamp.
74+
75+
Returns:
76+
The expiration time as an epoch timestamp, or sys.maxsize if the entry has no TTL.
77+
"""
4878
if value.ttl_at_insert is None:
4979
return float(sys.maxsize)
5080

@@ -56,7 +86,17 @@ def _memory_cache_ttu(_key: Any, value: MemoryCacheEntry, now: float) -> float:
5686

5787

5888
def _memory_cache_getsizeof(value: MemoryCacheEntry) -> int:
59-
"Return size of cache entry (always 1 for entry counting)."
89+
"""Return size of cache entry (always 1 for entry counting).
90+
91+
This function is used by TLRUCache to determine entry sizes. Since we want to count
92+
entries rather than measure memory usage, this always returns 1.
93+
94+
Args:
95+
value: The cache entry (unused).
96+
97+
Returns:
98+
Always returns 1 to enable entry-based size limiting.
99+
"""
60100
return 1
61101

62102

@@ -67,6 +107,12 @@ def _memory_cache_getsizeof(value: MemoryCacheEntry) -> int:
67107

68108

69109
class MemoryCollection:
110+
"""A fixed-size in-memory collection using TLRUCache.
111+
112+
This class wraps a time-aware LRU cache to provide TTL-based expiration
113+
and automatic eviction of old entries when the cache reaches its size limit.
114+
"""
115+
70116
_cache: TLRUCache[str, MemoryCacheEntry]
71117

72118
def __init__(self, max_entries: int = DEFAULT_MAX_ENTRIES_PER_COLLECTION):
@@ -78,6 +124,14 @@ def __init__(self, max_entries: int = DEFAULT_MAX_ENTRIES_PER_COLLECTION):
78124
self._cache = TLRUCache[str, MemoryCacheEntry](maxsize=max_entries, ttu=_memory_cache_ttu, getsizeof=_memory_cache_getsizeof)
79125

80126
def get(self, key: str) -> ManagedEntry | None:
127+
"""Retrieve an entry from the collection.
128+
129+
Args:
130+
key: The key to retrieve.
131+
132+
Returns:
133+
The ManagedEntry if found and not expired, None otherwise.
134+
"""
81135
managed_entry_str: MemoryCacheEntry | None = self._cache.get(key)
82136

83137
if managed_entry_str is None:
@@ -88,9 +142,25 @@ def get(self, key: str) -> ManagedEntry | None:
88142
return managed_entry
89143

90144
def put(self, key: str, value: ManagedEntry) -> None:
145+
"""Store an entry in the collection.
146+
147+
Args:
148+
key: The key to store under.
149+
value: The ManagedEntry to store.
150+
"""
151+
# Recalculate TTL to get remaining time until expiration
152+
value.recalculate_ttl()
91153
self._cache[key] = MemoryCacheEntry.from_managed_entry(managed_entry=value, ttl=value.ttl)
92154

93155
def delete(self, key: str) -> bool:
156+
"""Delete an entry from the collection.
157+
158+
Args:
159+
key: The key to delete.
160+
161+
Returns:
162+
True if the key was deleted, False if it didn't exist.
163+
"""
94164
return self._cache.pop(key, None) is not None
95165

96166
def keys(self, *, limit: int | None = None) -> list[str]:

key-value/key-value-sync/src/key_value/sync/code_gen/stores/windows_registry/utils.py

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,24 @@ def get_reg_sz_value(hive: HiveType, sub_key: str, value_name: str) -> str | Non
2626
return None
2727

2828

29-
def set_reg_sz_value(hive: HiveType, sub_key: str, value_name: str, value: str) -> None:
29+
def set_reg_sz_value(hive: HiveType, sub_key: str, value_name: str, value: str) -> bool:
3030
"""Set a string value in the Windows Registry.
3131
3232
Args:
3333
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
3434
sub_key: The registry subkey path.
3535
value_name: The name of the registry value to set.
3636
value: The string value to write.
37+
38+
Returns:
39+
True if the value was set, False if the key doesn't exist or couldn't be accessed.
3740
"""
38-
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
39-
winreg.SetValueEx(reg_key, value_name, 0, winreg.REG_SZ, value)
41+
try:
42+
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
43+
winreg.SetValueEx(reg_key, value_name, 0, winreg.REG_SZ, value)
44+
return True
45+
except (FileNotFoundError, OSError):
46+
return False
4047

4148

4249
def delete_reg_sz_value(hive: HiveType, sub_key: str, value_name: str) -> bool:
@@ -75,14 +82,22 @@ def has_key(hive: HiveType, sub_key: str) -> bool:
7582
return False
7683

7784

78-
def create_key(hive: HiveType, sub_key: str) -> None:
85+
def create_key(hive: HiveType, sub_key: str) -> bool:
7986
"""Create a new registry key.
8087
8188
Args:
8289
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
8390
sub_key: The registry subkey path to create.
91+
92+
Returns:
93+
True if the key was created or already exists, False on error.
8494
"""
85-
winreg.CreateKey(hive, sub_key)
95+
try:
96+
winreg.CreateKey(hive, sub_key)
97+
except OSError:
98+
return False
99+
else:
100+
return True
86101

87102

88103
def delete_key(hive: HiveType, sub_key: str) -> bool:
@@ -96,32 +111,34 @@ def delete_key(hive: HiveType, sub_key: str) -> bool:
96111
True if the key was deleted, False if it didn't exist or couldn't be deleted.
97112
"""
98113
try:
99-
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE) as reg_key:
100-
winreg.DeleteKey(reg_key, sub_key)
101-
return True
114+
winreg.DeleteKey(hive, sub_key)
102115
except (FileNotFoundError, OSError):
103116
return False
117+
else:
118+
return True
104119

105120

106121
def delete_sub_keys(hive: HiveType, sub_key: str) -> None:
107-
"""Delete all subkeys of a registry key.
122+
"""Delete all immediate subkeys of a registry key.
108123
109-
This function recursively deletes all subkeys under the specified registry key.
124+
This function deletes all immediate child keys (non-recursive).
110125
111126
Args:
112127
hive: The registry hive (e.g., winreg.HKEY_CURRENT_USER).
113128
sub_key: The registry subkey path whose subkeys should be deleted.
114129
"""
115130
try:
116131
with winreg.OpenKey(key=hive, sub_key=sub_key, access=winreg.KEY_WRITE | winreg.KEY_ENUMERATE_SUB_KEYS) as reg_key:
117-
index = 0
118132
while True:
119-
if not (next_child_key := winreg.EnumKey(reg_key, index)):
133+
try:
134+
# Always enumerate at index 0 since keys shift after deletion
135+
next_child_key = winreg.EnumKey(reg_key, 0)
136+
except OSError:
137+
# No more subkeys
120138
break
121139

122-
with contextlib.suppress(Exception):
140+
# Key already deleted or can't be deleted, skip it
141+
with contextlib.suppress(FileNotFoundError, OSError):
123142
winreg.DeleteKey(reg_key, next_child_key)
124-
125-
index += 1
126143
except (FileNotFoundError, OSError):
127144
return

0 commit comments

Comments
 (0)