From 9fd54e86e3df269369cbebd1f8b87fdc38bbd50b Mon Sep 17 00:00:00 2001 From: CaptainAni187 Date: Tue, 8 Sep 2026 19:50:25 +0530 Subject: [PATCH 1/2] fix(ndb): persist updates to existing dynamic Expando properties `Expando.__setattr__` delegated to `object.__setattr__` whenever the name was already present in `_properties`. That is only correct for names backed by a class level descriptor, because the descriptor is what writes `_values`. A dynamic property has no descriptor, so the value landed in `__dict__` while `_values` kept the previous value, and `put()` serialized the stale one. Reading the attribute back in the same process returned the new value, since `__dict__` shadows `__getattr__`, so the write appeared to succeed and only a refetch revealed the old value. Restrict the delegation to class level descriptors, which is the same line `__delattr__` already draws. --- .../google/cloud/ndb/model.py | 11 ++++++---- .../google-cloud-ndb/tests/unit/test_model.py | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/google-cloud-ndb/google/cloud/ndb/model.py b/packages/google-cloud-ndb/google/cloud/ndb/model.py index 8d19dded179a..c672f481b4ed 100644 --- a/packages/google-cloud-ndb/google/cloud/ndb/model.py +++ b/packages/google-cloud-ndb/google/cloud/ndb/model.py @@ -6363,11 +6363,14 @@ def __getattr__(self, name): def __setattr__(self, name, value): if self._properties is None: raise TypeError("self._properties cannot be None") - if ( - name.startswith("_") - or isinstance(getattr(self.__class__, name, None), (Property, property)) - or isinstance(self._properties.get(name, None), (Property, property)) + if name.startswith("_") or isinstance( + getattr(self.__class__, name, None), (Property, property) ): + # Only names backed by a class level descriptor can be delegated to + # ``object.__setattr__``: the descriptor is what writes ``_values``. + # A dynamic property has no descriptor, so delegating would put the + # value in ``__dict__`` and leave ``_values`` stale, which is what + # ``put()`` serializes. ``__delattr__`` draws the same line. return super(Expando, self).__setattr__(name, value) if "." in name: diff --git a/packages/google-cloud-ndb/tests/unit/test_model.py b/packages/google-cloud-ndb/tests/unit/test_model.py index 6b5e3fbbc059..e143bf530f27 100644 --- a/packages/google-cloud-ndb/tests/unit/test_model.py +++ b/packages/google-cloud-ndb/tests/unit/test_model.py @@ -6175,6 +6175,27 @@ class Expansive(model.Expando): assert expansive.a.b == "one" assert expansive.a.c == "two" + @staticmethod + def test___setattr__updates_dynamic_property(): + """Regression test for issue #18204 + + Re-assigning a dynamic property must update ``_values``, which is what + ``put()`` serializes, rather than shadowing it in ``__dict__``. + """ + + class Expansive(model.Expando): + foo = model.StringProperty() + + expansive = Expansive(foo="x") + + expansive.bar = 2.0 + assert expansive._values["bar"] == 2.0 + + expansive.bar = 9.99 + assert expansive.bar == 9.99 + assert expansive._values["bar"] == 9.99 + assert "bar" not in expansive.__dict__ + @staticmethod def test___delattr__(): class Expansive(model.Expando): From e15738b81c0239793a24e081a40139de87b8375c Mon Sep 17 00:00:00 2001 From: CaptainAni187 Date: Tue, 8 Sep 2026 20:07:05 +0530 Subject: [PATCH 2/2] chore: re-trigger CLA check