fix: fix bug with repeated structured properties with Expando values (#…

…671)

In the legacy data format, the dotted properties stored in Datastore
were not properly padded for missing values.

Fixes #669

google/cloud/ndb/model.py

@@ -696,26 +696,29 @@ def _entity_from_protobuf(protobuf):
     return _entity_from_ds_entity(ds_entity)
 
 
-def _properties_of(entity):
-    """Get the model properties for an entity.
+def _properties_of(*entities):
+    """Get the model properties for one or more entities.
 
-    After collecting any properties local to the given entity, will traverse the
-    entity's MRO (class hierarchy) up from the entity's class through all of its
-    ancestors, collecting an ``Property`` instances defined for those classes.
+    After collecting any properties local to the given entities, will traverse the
+    entities' MRO (class hierarchy) up from the entities' class through all of its
+    ancestors, collecting any ``Property`` instances defined for those classes.
 
     Args:
-        entity (model.Model): The entity to get properties for.
+        entities (Tuple[model.Model]): The entities to get properties for. All entities
+            are expected to be of the same class.
 
     Returns:
-        Iterator[Property]: Iterator over the entity's properties.
+        Iterator[Property]: Iterator over the entities' properties.
     """
     seen = set()
 
-    for level in (entity,) + tuple(type(entity).mro()):
+    entity_type = type(entities[0])  # assume all entities are same type
+    for level in entities + tuple(entity_type.mro()):
         if not hasattr(level, "_properties"):
             continue
 
-        for prop in level._properties.values():
+        level_properties = getattr(level, "_properties", {})
+        for prop in level_properties.values():
             if (
                 not isinstance(prop, Property)
                 or isinstance(prop, ModelKey)
@@ -4299,6 +4302,8 @@ def _to_datastore(self, entity, data, prefix="", repeated=False):
         if not self._repeated:
             values = (values,)
 
+        props = tuple(_properties_of(*values))
+
         for value in values:
             if value is None:
                 keys.extend(
@@ -4308,7 +4313,7 @@ def _to_datastore(self, entity, data, prefix="", repeated=False):
                 )
                 continue
 
-            for prop in _properties_of(value):
+            for prop in props:
                 keys.extend(
                     prop._to_datastore(
                         value, data, prefix=next_prefix, repeated=next_repeated

tests/system/test_crud.py

@@ -1117,6 +1117,53 @@ class SomeKind(ndb.Model):
     assert isinstance(retrieved.bar[1], OtherKind)
 
 
+@pytest.mark.usefixtures("client_context")
+def test_legacy_repeated_structured_property_w_expando(
+    ds_client, dispose_of, client_context
+):
+    """Regression test for #669
+
+    https://github.com/googleapis/python-ndb/issues/669
+    """
+
+    class OtherKind(ndb.Expando):
+        one = ndb.StringProperty()
+
+    class SomeKind(ndb.Model):
+        foo = ndb.IntegerProperty()
+        bar = ndb.StructuredProperty(OtherKind, repeated=True)
+
+    entity = SomeKind(
+        foo=42,
+        bar=[
+            OtherKind(one="one-a"),
+            OtherKind(two="two-b"),
+            OtherKind(one="one-c", two="two-c"),
+        ],
+    )
+
+    with client_context.new(legacy_data=True).use():
+        key = entity.put()
+        dispose_of(key._key)
+
+        ds_entity = ds_client.get(key._key)
+        assert ds_entity["bar.one"] == ["one-a", None, "one-c"]
+        assert ds_entity["bar.two"] == [None, "two-b", "two-c"]
+
+        retrieved = key.get()
+        assert retrieved.foo == 42
+        assert retrieved.bar[0].one == "one-a"
+        assert not hasattr(retrieved.bar[0], "two")
+        assert retrieved.bar[1].one is None
+        assert retrieved.bar[1].two == "two-b"
+        assert retrieved.bar[2].one == "one-c"
+        assert retrieved.bar[2].two == "two-c"
+
+        assert isinstance(retrieved.bar[0], OtherKind)
+        assert isinstance(retrieved.bar[1], OtherKind)
+        assert isinstance(retrieved.bar[2], OtherKind)
+
+
 @pytest.mark.usefixtures("client_context")
 def test_insert_expando(dispose_of):
     class SomeKind(ndb.Expando):