fix(abstractions): pass mypy --strict in store and fix the backing store bugs it shows

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,15 +17,15 @@ 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":
                 field_val = getattr(self, field.name)
                 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:

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]):

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,43 +15,42 @@ 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:
         """Returns the set backing store"""
         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

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
 

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,22 +27,22 @@ 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
                 if backing_store:
                     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

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 @@
     @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,18 +72,16 @@
 
         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]
             ):
                 return result_value
             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 @@
             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 @@
 
         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 @@
 
         # 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,26 +181,28 @@
         """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:
                     # Call get() on nested properties so that this method may be called recursively
                     # 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_()

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())

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")]