From 650235a1723f7c272fd445d411c5a11f5208b919 Mon Sep 17 00:00:00 2001 From: HardMax71 Date: Wed, 14 Oct 2026 21:06:26 +0200 Subject: [PATCH] fix(abstractions): pass mypy --strict in store and fix the backing store bugs it shows Annotate the rest of kiota_abstractions.store and default the BackingStore and InMemoryBackingStore type parameters to object, like RequestAdapter and ParsableFactory, so generated models that declare a bare `backing_store: BackingStore` still pass mypy --strict. BackingStoreFactorySingleton was never a singleton. `hasattr(cls, "__instance")` checks the unmangled name, so every call built a new instance and assigned the factory over the `backing_store_factory` property on the class, and a call with None put back a fresh InMemoryBackingStoreFactory. Generated models call it with None at import, so a factory passed to enable_backing_store never stuck. It now keeps one instance, None keeps the current factory, and a factory replaces it. InMemoryBackingStore stored lists as a (list, size) tuple and get() unwrapped any tuple, so a stored tuple such as (1, 2) came back as 1. Entries now keep the value and the list's size side by side, so get() returns what was set. enumerate_() still returns the (list, size) pair for lists. Subscribers now get the previous and new values, as subscribe documents, instead of the internal entries. Part of #722. --- .../kiota_abstractions/store/backed_model.py | 5 +- .../kiota_abstractions/store/backing_store.py | 6 +- .../store/backing_store_factory_singleton.py | 45 ++++++------ .../store/backing_store_parse_node_factory.py | 6 +- ...tore_serialization_writer_proxy_factory.py | 13 ++-- .../store/in_memory_backing_store.py | 70 +++++++++++-------- .../test_backing_store_factory_singleton.py | 14 +++- .../store/test_in_memory_backing_store.py | 13 ++++ 8 files changed, 105 insertions(+), 67 deletions(-) diff --git a/packages/abstractions/kiota_abstractions/store/backed_model.py b/packages/abstractions/kiota_abstractions/store/backed_model.py index 8d3325e0..39660636 100644 --- a/packages/abstractions/kiota_abstractions/store/backed_model.py +++ b/packages/abstractions/kiota_abstractions/store/backed_model.py @@ -5,6 +5,7 @@ # ------------------------------------------------------------------------------ from dataclasses import dataclass, fields +from typing import Any from .backing_store import BackingStore @@ -16,7 +17,7 @@ class BackedModel: # Stores model information. backing_store: BackingStore - def __post_init__(self): + def __post_init__(self) -> None: self.backing_store.is_initialization_completed = True for field in fields(self): if field.name != "backing_store": @@ -24,7 +25,7 @@ def __post_init__(self): if field_val is not field.default: self.backing_store.set(field.name, field_val) - def __setattr__(self, prop, val): + def __setattr__(self, prop: str, val: Any) -> None: if prop == "backing_store": super().__setattr__(prop, val) else: diff --git a/packages/abstractions/kiota_abstractions/store/backing_store.py b/packages/abstractions/kiota_abstractions/store/backing_store.py index 5125beb6..ce3256f7 100644 --- a/packages/abstractions/kiota_abstractions/store/backing_store.py +++ b/packages/abstractions/kiota_abstractions/store/backing_store.py @@ -6,9 +6,11 @@ from abc import ABC, abstractmethod from collections.abc import Callable -from typing import Any, Generic, Optional, TypeVar +from typing import Any, Generic, Optional -T = TypeVar("T") +from typing_extensions import TypeVar + +T = TypeVar("T", default=object) class BackingStore(ABC, Generic[T]): diff --git a/packages/abstractions/kiota_abstractions/store/backing_store_factory_singleton.py b/packages/abstractions/kiota_abstractions/store/backing_store_factory_singleton.py index 052f251f..e79f1de4 100644 --- a/packages/abstractions/kiota_abstractions/store/backing_store_factory_singleton.py +++ b/packages/abstractions/kiota_abstractions/store/backing_store_factory_singleton.py @@ -4,6 +4,8 @@ # See License in the project root for license information. # ------------------------------------------------------------------------------ +from typing import Optional + from kiota_abstractions.store.backing_store_factory import BackingStoreFactory from kiota_abstractions.store.in_memory_backing_store_factory import InMemoryBackingStoreFactory @@ -13,36 +15,35 @@ class BackingStoreFactorySingleton: Ensures that there is only a single instance of a Backing Store Factory. """ - def __new__(cls, *args, **kwargs): + __instance: Optional["BackingStoreFactorySingleton"] = None + _backing_store_factory: BackingStoreFactory + + def __new__( + cls, + backing_store_factory: Optional[BackingStoreFactory] = None + ) -> "BackingStoreFactorySingleton": """ - Creates a single instance of the class during instantiation. It also - ensures that the backing store factory that was set in the first - instantion of the class will be the default during run time. + Returns the single instance of the class, created on first use with an + InMemoryBackingStoreFactory. """ - try: - bs_arg = args[0] - except IndexError: - bs_arg = kwargs.get("backing_store_factory", None) - - bs_property = hasattr(cls, "backing_store_factory") - if not hasattr(cls, "__instance"): - if bs_property and isinstance(bs_arg, BackingStoreFactory): - cls.backing_store_factory = bs_arg - else: - # Default backing store is InMemoryBackingStoreFactory - cls.backing_store_factory = InMemoryBackingStoreFactory() - cls.__instance = super(BackingStoreFactorySingleton, cls).__new__(cls) + if cls.__instance is None: + cls.__instance = super().__new__(cls) + cls.__instance._backing_store_factory = InMemoryBackingStoreFactory() return cls.__instance @classmethod - def get_instance(cls): + def get_instance(cls) -> "BackingStoreFactorySingleton": """ Returns the instance of the class. """ - return cls.__instance + return cls() - def __init__(self, backing_store_factory: BackingStoreFactory) -> None: - self._backing_store_factory = backing_store_factory + def __init__(self, backing_store_factory: Optional[BackingStoreFactory] = None) -> None: + """ + Replaces the backing store factory when one is given. None keeps the current one. + """ + if backing_store_factory is not None: + self._backing_store_factory = backing_store_factory @property def backing_store_factory(self) -> BackingStoreFactory: @@ -50,6 +51,6 @@ def backing_store_factory(self) -> BackingStoreFactory: return self._backing_store_factory @backing_store_factory.setter - def backing_store_factory(self, backing_store_factory: BackingStoreFactory): + def backing_store_factory(self, backing_store_factory: BackingStoreFactory) -> None: """Sets the backing store to the value initialized in the singleton.""" self._backing_store_factory = backing_store_factory diff --git a/packages/abstractions/kiota_abstractions/store/backing_store_parse_node_factory.py b/packages/abstractions/kiota_abstractions/store/backing_store_parse_node_factory.py index cb611476..c6c376c7 100644 --- a/packages/abstractions/kiota_abstractions/store/backing_store_parse_node_factory.py +++ b/packages/abstractions/kiota_abstractions/store/backing_store_parse_node_factory.py @@ -4,7 +4,7 @@ # See License in the project root for license information. # ------------------------------------------------------------------------------ -from ..serialization import ParseNodeFactory, ParseNodeProxyFactory +from ..serialization import Parsable, ParseNodeFactory, ParseNodeProxyFactory from .backed_model import BackedModel @@ -18,11 +18,11 @@ def __init__(self, concrete: ParseNodeFactory) -> None: implementation ParseNodeFactory. """ - def on_before_deserialization(x): + def on_before_deserialization(x: Parsable) -> None: if isinstance(x, BackedModel) and x.backing_store: x.backing_store.is_initialization_completed = False - def on_after_deserialization(x): + def on_after_deserialization(x: Parsable) -> None: if isinstance(x, BackedModel) and x.backing_store: x.backing_store.is_initialization_completed = True diff --git a/packages/abstractions/kiota_abstractions/store/backing_store_serialization_writer_proxy_factory.py b/packages/abstractions/kiota_abstractions/store/backing_store_serialization_writer_proxy_factory.py index fe27b878..c3cc748f 100644 --- a/packages/abstractions/kiota_abstractions/store/backing_store_serialization_writer_proxy_factory.py +++ b/packages/abstractions/kiota_abstractions/store/backing_store_serialization_writer_proxy_factory.py @@ -4,7 +4,12 @@ # See License in the project root for license information. # ------------------------------------------------------------------------------ -from ..serialization import SerializationWriterFactory, SerializationWriterProxyFactory +from ..serialization import ( + Parsable, + SerializationWriter, + SerializationWriterFactory, + SerializationWriterProxyFactory, +) from .backed_model import BackedModel @@ -22,14 +27,14 @@ def __init__(self, concrete: SerializationWriterFactory) -> None: SerializationWriterFactory to wrap. """ - def on_before(x): + def on_before(x: Parsable) -> None: if isinstance(x, BackedModel): backed_model = x backing_store = backed_model.backing_store if backing_store: backing_store.return_only_changed_values = True - def on_after(x): + def on_after(x: Parsable) -> None: if isinstance(x, BackedModel): backed_model = x backing_store = backed_model.backing_store @@ -37,7 +42,7 @@ def on_after(x): backing_store.return_only_changed_values = False backing_store.is_initialization_completed = True - def on_start(x, y): + def on_start(x: Parsable, y: SerializationWriter) -> None: if isinstance(x, BackedModel): backed_model = x backing_store = backed_model.backing_store diff --git a/packages/abstractions/kiota_abstractions/store/in_memory_backing_store.py b/packages/abstractions/kiota_abstractions/store/in_memory_backing_store.py index 77590199..fef8c809 100644 --- a/packages/abstractions/kiota_abstractions/store/in_memory_backing_store.py +++ b/packages/abstractions/kiota_abstractions/store/in_memory_backing_store.py @@ -5,22 +5,30 @@ # ------------------------------------------------------------------------------ from collections.abc import Callable -from typing import Any, Generic, Optional, TypeVar +from typing import Any, Generic, NamedTuple, Optional from uuid import uuid4 +from typing_extensions import TypeVar + from .backed_model import BackedModel from .backing_store import BackingStore -T = TypeVar("T") +T = TypeVar("T", default=object) + + +class _Collection(NamedTuple): + """A stored list and its size when it was set, so a size change marks it dirty.""" + items: list[Any] + size: int -class InMemoryBackingStore(BackingStore, Generic[T]): +class InMemoryBackingStore(BackingStore[T], Generic[T]): """In-memory implementation of the backing store. Allows for dirty tracking of changes.""" def __init__(self) -> None: self.__subscriptions: dict[str, Callable[[str, Any, Any], None]] = {} - self.__store: dict[str, tuple[bool, Any]] = {} + self.__store: dict[str, tuple[bool, T, Optional[_Collection]]] = {} self.__initialization_completed: bool = False self.__return_only_changed_values: bool = False @@ -32,12 +40,12 @@ def is_initialization_completed(self) -> bool: @is_initialization_completed.setter def is_initialization_completed(self, value: bool) -> None: self.__initialization_completed = value - for key, val in self.__store.items(): - if isinstance(val[1], BackedModel): + for key, (_, val, collection) in self.__store.items(): + if isinstance(val, BackedModel): # forward the initialization status to nested BackedModel instances - val[1].backing_store.is_initialization_completed = value - self._ensure_collection_size_is_consistent(key, val[1]) - self.__store[key] = (not value, val[1]) + val.backing_store.is_initialization_completed = value + self._ensure_collection_size_is_consistent(key, val, collection) + self.__store[key] = (not value, val, collection) @property def return_only_changed_values(self) -> bool: @@ -64,10 +72,8 @@ def get(self, key: str) -> Optional[T]: result = self.__store.get(key) if result: - self._ensure_collection_size_is_consistent(key, result[1]) - result_value = result[1] - if isinstance(result_value, tuple): - result_value = result_value[0] + _, result_value, collection = result + self._ensure_collection_size_is_consistent(key, result_value, collection) if self.return_only_changed_values is False or ( self.return_only_changed_values is True and self.__store[key][0] ): @@ -75,7 +81,7 @@ def get(self, key: str) -> Optional[T]: return None return None - def set(self, key: str, value: Any) -> None: + def set(self, key: str, value: T) -> None: """Sets the specified object with the given key in the store. Args: @@ -86,9 +92,8 @@ def set(self, key: str, value: Any) -> None: raise ValueError("Key cannot be empty or None") old_value = self.__store.get(key) - value_to_add = (self.is_initialization_completed, value) - if isinstance(value, list): - value_to_add = (self.is_initialization_completed, (value, len(value))) # type: ignore + collection = _Collection(value, len(value)) if isinstance(value, list) else None + value_to_add = (self.is_initialization_completed, value, collection) if key not in self.__store: if isinstance(value, BackedModel) and value.backing_store: @@ -110,7 +115,7 @@ def set(self, key: str, value: Any) -> None: self.__store[key] = value_to_add for sub in list(self.__subscriptions): - self.__subscriptions[sub](key, old_value, value_to_add) + self.__subscriptions[sub](key, None if old_value is None else old_value[1], value) # sub(key, old_value, value_to_add) def enumerate_(self) -> list[tuple[str, Any]]: @@ -124,13 +129,14 @@ def enumerate_(self) -> list[tuple[str, Any]]: # refresh the state of collection properties if they've changed in size. if self.return_only_changed_values: - for key, val in self.__store.items(): - self._ensure_collection_size_is_consistent(key, val[1]) + for key, (_, val, collection) in self.__store.items(): + self._ensure_collection_size_is_consistent(key, val, collection) - keyval_pairs = list(self.__store.items()) - if self.return_only_changed_values: - return [(key, val[1]) for key, val in keyval_pairs if val[0] is True] - return [(key, val[1]) for key, val in keyval_pairs] + return [ + (key, val if collection is None else collection) + for key, (changed, val, collection) in list(self.__store.items()) + if changed or not self.return_only_changed_values + ] def enumerate_keys_for_values_changed_to_null(self) -> list[str]: """Enumerate the values in the store that have changed to None @@ -175,17 +181,19 @@ def clear(self) -> None: """Clears the store""" self.__store.clear() - def _ensure_collection_size_is_consistent(self, key: str, entry: Any) -> None: + def _ensure_collection_size_is_consistent( + self, key: str, entry: T, collection: Optional[_Collection] + ) -> None: """Checks if entry of type collection has changed in size. If so, dirty tracks the change by calling set() Args: key (str): _description_ - entry(StoreEntry): _description_ + entry (T): the stored value + collection (Optional[_Collection]): the stored list and its size, if entry is a list """ - # Check if the entry is a tuple of a collection annotated with the size - if isinstance(entry, tuple) and isinstance(entry[0], list) and isinstance(entry[1], int): - backed_models = [item for item in entry[0] if isinstance(item, BackedModel)] + if collection is not None: + backed_models = [item for item in collection.items if isinstance(item, BackedModel)] for backed_model in backed_models: values = backed_model.backing_store.enumerate_() for value in values: @@ -193,8 +201,8 @@ def _ensure_collection_size_is_consistent(self, key: str, entry: Any) -> None: # to ensure collections are consistent backed_model.backing_store.get(value[0]) - if len(entry[0]) != entry[1]: # if the size has changed since last update - self.set(key, entry[0]) # dirty track the change + if len(collection.items) != collection.size: + self.set(key, entry) # dirty track the change elif isinstance(entry, BackedModel): values = entry.backing_store.enumerate_() diff --git a/packages/abstractions/tests/store/test_backing_store_factory_singleton.py b/packages/abstractions/tests/store/test_backing_store_factory_singleton.py index 6452cef2..b284f8a6 100644 --- a/packages/abstractions/tests/store/test_backing_store_factory_singleton.py +++ b/packages/abstractions/tests/store/test_backing_store_factory_singleton.py @@ -73,9 +73,7 @@ def test_backing_store_factory_default(): def test_backing_store_factory_is_singleton(): """ - Test that the backing store factory that was initialized with the backing - store singleton is not overridden when you initialize backing store factory - singleton with a different backing store factory at runtime. + Test that every instantiation returns the same instance and backing store factory. """ bsf = BackingStoreFactorySingleton( backing_store_factory=MockTestBackingStoreFactory() @@ -85,3 +83,13 @@ def test_backing_store_factory_is_singleton(): ) assert bsf.get_instance() == bsf2.get_instance() assert bsf.backing_store_factory is bsf2.backing_store_factory + + +def test_backing_store_factory_kept_when_none_is_passed(): + custom = MockTestBackingStoreFactory() + try: + bsf = BackingStoreFactorySingleton(backing_store_factory=custom) + assert BackingStoreFactorySingleton(backing_store_factory=None) is bsf + assert bsf.backing_store_factory is custom + finally: + BackingStoreFactorySingleton(backing_store_factory=InMemoryBackingStoreFactory()) diff --git a/packages/abstractions/tests/store/test_in_memory_backing_store.py b/packages/abstractions/tests/store/test_in_memory_backing_store.py index 269d0916..dc08615a 100644 --- a/packages/abstractions/tests/store/test_in_memory_backing_store.py +++ b/packages/abstractions/tests/store/test_in_memory_backing_store.py @@ -215,3 +215,16 @@ def test_backing_store_embedded_in_model_by_updating_nested_backed_model_collect + +def test_stores_tuple_values_as_they_are(): + backing_store = InMemoryBackingStore() + backing_store.set("coordinates", (1, 2)) + assert backing_store.get("coordinates") == (1, 2) + +def test_subscribers_get_previous_and_new_values(): + backing_store = InMemoryBackingStore() + changes = [] + backing_store.subscribe(lambda key, old, new: changes.append((key, old, new))) + backing_store.set("name", "Samwel") + backing_store.set("name", "Bill") + assert changes == [("name", None, "Samwel"), ("name", "Samwel", "Bill")]