Skip to content

fix(abstractions): subscribe list items once and keep list sizes fresh in the backing store - #802

Open
Max Azatian (HardMax71) wants to merge 2 commits into
microsoft:mainfrom
HardMax71:fix/backing-store-subscriptions-and-sizes
Open

Max Azatian (HardMax71) wants to merge 2 commits into
microsoft:mainfrom
HardMax71:fix/backing-store-subscriptions-and-sizes

Conversation

@HardMax71

@HardMax71 Max Azatian (HardMax71) commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Two more backing store bugs, found while typing the store in #800. This branch builds on #800, so until that merges the diff also shows its commit. The change here is only the last commit, 723f291.

  • Setting a list gave each BackedModel in it one more subscription. Each one sets the list again, so every change to an item doubled its subscriptions, 256 after 8 changes. Items now subscribe under the store's own id plus the property name. A repeat subscription from the same store replaces the old one, and a child shared by two stores still notifies both.
  • The is_initialization_completed setter saved the entry it read before the size check. A list that had grown was saved with its old size and showed up as changed right after. It now saves the entry after the check, like the C# store.

Related Issue

Follow-up to #800, found while working on #722.

Testing Instructions

  • pytest in packages/abstractions passes.
  • Without the source change, test_list_items_notify_the_store_once_per_change and test_completing_initialization_clears_a_resized_list fail.
  • test_list_item_shared_by_two_stores_notifies_both covers a child shared by two stores.

…ore 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 microsoft#722.
@HardMax71
Max Azatian (HardMax71) requested a review from a team as a code owner October 14, 2026 19:16
Copilot AI balanced review requested due to automatic review settings October 14, 2026 19:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Subscription IDs collide across parent stores sharing a child, causing missed change notifications.

1 open finding

🧠 Review effort: Balanced

item.backing_store.is_initialization_completed = True
item.backing_store.subscribe(
lambda prop_key, old_val, new_val: self.set(key, value)
lambda prop_key, old_val, new_val: self.set(key, value), key

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't thought about it tbh, fixed in 723f291

…h in the backing store

Each time a list was set, every BackedModel in it got one more subscription. The callback sets the list again, so each change to an item doubled its subscriptions. Items now subscribe under the store's own id plus the property name. A repeat subscription from the same store replaces the old one, and a child shared by two stores still notifies both.

The is_initialization_completed setter saved the entry it read before the size check. A list that had grown was saved with its old size and showed up as changed again on the next enumerate_. It now saves the entry after the check, as the C# store does.
@HardMax71
Max Azatian (HardMax71) force-pushed the fix/backing-store-subscriptions-and-sizes branch from 7c50a5b to 723f291 Compare October 14, 2026 19:25
Copilot AI balanced review requested due to automatic review settings October 14, 2026 19:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants