fix(ndb): persist updates to existing dynamic Expando properties

[CaptainAni187]

Fixes #18204

Re-assigning a dynamic property on an Expando did not persist. The first write stuck; later writes looked fine in process but a refetch returned the original value.

Cause

Expando.__setattr__ delegated to object.__setattr__ in three cases:

if (
    name.startswith("_")
    or isinstance(getattr(self.__class__, name, None), (Property, property))
    or isinstance(self._properties.get(name, None), (Property, property))
):
    return super(Expando, self).__setattr__(name, value)

The third case is the problem. Delegating is only correct for a name backed by a class level descriptor, because the descriptor's __set__ is what writes _values. A dynamic property lives in _properties but has no descriptor on the class, so after the first assignment put it there, every later assignment took this branch and landed in __dict__. _values kept the old value, and that is what put() serializes.

It also explains why the write appeared to succeed: __dict__ shadows __getattr__, so reading the attribute back in the same process returned the new value.

Without Datastore:

class Item(ndb.Expando):
    name = ndb.StringProperty()

e = Item(name="x")
e.extra = 2.0
e.extra = 9.99

e.extra              # 9.99  (read from __dict__)
e._values["extra"]   # 2.0   (what put() writes)

Declared properties were unaffected because they hit the descriptor via the second condition.

Change

Restrict the delegation to class level descriptors. This is the same line __delattr__ immediately below already draws — it checks only name.startswith("_") and the class attribute, then routes everything else through the property. The two now agree.

The third condition arrived in 9be06e5 ("properly handle legacy structured properties in Expando instances", #676, fixing googleapis/python-ndb#673) alongside the dotted-name handling. That commit's regression test, test___setattr__with_dotted_name, still passes — dotted names resolve through the branch below and never needed this condition.

Tests

Adds test___setattr__updates_dynamic_property, asserting that a re-assigned dynamic property updates _values and does not leave a shadowing entry in __dict__. It fails without the change.

Full unit suite: 1834 passed, 1 skipped (1833 before, plus the new test). ruff check, ruff format and flake8 clean at the pinned 0.14.14.

I verified the behaviour at the _values level rather than through a real put()/get() round trip, since that needs a Datastore instance — happy to add a system test under tests/system/ if you would prefer one there.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request fixes a bug in Expando.__setattr__ where re-assigning a dynamic property would incorrectly delegate to super().__setattr__ and shadow the value in __dict__ instead of updating _values. The fix removes the check for instance properties in self._properties during delegation, ensuring dynamic properties are correctly serialized. A regression unit test has also been added to verify this behavior. There are no review comments, so I have no feedback to provide.

[CaptainAni187]

@googlebot I signed it!