Skip to content

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

Merged
Vincent Biret (baywet) merged 4 commits into
microsoft:mainfrom
HardMax71:fix/backing-store-subscriptions-and-sizes
Oct 15, 2026
Merged

Vincent Biret (baywet) merged 4 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.

  • 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.

@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

This comment was marked as off-topic.

This comment was marked as outdated.

…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

This comment was marked as off-topic.

This comment was marked as outdated.

@baywet

Copy link
Copy Markdown
Member

Max Azatian (@HardMax71) can you look into the failing checks please?

@HardMax71

Max Azatian (HardMax71) commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

Max Azatian (Max Azatian (@HardMax71)) can you look into the failing checks please?

Vincent Biret (@baywet) fixed in 0fa3b1f. pylint flagged reading store[key] inside the items() loop. The setter now loops over the keys, the same as the C# one.

pylint R1733 flagged reading store[key] inside the items() loop. The size
check can replace the entry, so the loop now goes over a copy of the keys
and reads each entry from the store, like the C# setter.
Copilot AI balanced review requested due to automatic review settings October 15, 2026 11:43

This comment was marked as off-topic.

@baywet
Vincent Biret (baywet) requested a balanced review from Copilot October 15, 2026 12:15

@baywet Vincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for making the changes!

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.

🟢 Approval recommended

The focused fixes address both reported defects with targeted regression coverage and no unresolved findings.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:18

This comment was marked as off-topic.

@sonarqubecloud

Copy link
Copy Markdown

@baywet
Vincent Biret (baywet) merged commit 76720dc into microsoft:main Oct 15, 2026
63 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress 🚧 to Done ✔️ in Kiota Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done ✔️

Development

Successfully merging this pull request may close these issues.

3 participants