From 0bdcbcaab4974ba95765ab5e00e967542e4ce886 Mon Sep 17 00:00:00 2001 From: Chris Rossi Date: Fri, 30 Aug 2019 16:47:38 -0400 Subject: [PATCH] Backwards compatibility for structured props. --- google/cloud/ndb/client.py | 4 + google/cloud/ndb/context.py | 3 + google/cloud/ndb/model.py | 122 ++++++++++++++--- tests/system/conftest.py | 5 +- tests/system/test_crud.py | 30 +++++ tests/system/test_query.py | 260 ++++++++++++++++++++++++++++++++++++ 6 files changed, 401 insertions(+), 23 deletions(-) diff --git a/google/cloud/ndb/client.py b/google/cloud/ndb/client.py index 5b195a48..ba9709bd 100644 --- a/google/cloud/ndb/client.py +++ b/google/cloud/ndb/client.py @@ -114,6 +114,7 @@ def context( global_cache=None, global_cache_policy=None, global_cache_timeout_policy=None, + legacy_data=True, ): """Establish a context for a set of NDB calls. @@ -157,6 +158,8 @@ def context( global_cache_timeout_policy (Optional[Callable[[key.Key], int]]): The global cache timeout to use in this context. See: :meth:`~google.cloud.ndb.context.Context.set_global_cache_timeout_policy`. + legacy_data (bool): Set to ``True`` (the default) to write data in + a way that can be read by the legacy version of NDB. """ context = context_module.Context( self, @@ -164,6 +167,7 @@ def context( global_cache=global_cache, global_cache_policy=global_cache_policy, global_cache_timeout_policy=global_cache_timeout_policy, + legacy_data=legacy_data, ) with context.use(): yield context diff --git a/google/cloud/ndb/context.py b/google/cloud/ndb/context.py index ebb92aa1..54fa80e9 100644 --- a/google/cloud/ndb/context.py +++ b/google/cloud/ndb/context.py @@ -144,6 +144,7 @@ def policy(key): "cache", "global_cache", "on_commit_callbacks", + "legacy_data", ], ) @@ -180,6 +181,7 @@ def __new__( global_cache_timeout_policy=None, datastore_policy=None, on_commit_callbacks=None, + legacy_data=True, ): if eventloop is None: eventloop = _eventloop.EventLoop() @@ -210,6 +212,7 @@ def __new__( cache=new_cache, global_cache=global_cache, on_commit_callbacks=on_commit_callbacks, + legacy_data=legacy_data, ) context.set_cache_policy(cache_policy) diff --git a/google/cloud/ndb/model.py b/google/cloud/ndb/model.py index 9b4f7be5..4dc57ac4 100644 --- a/google/cloud/ndb/model.py +++ b/google/cloud/ndb/model.py @@ -646,6 +646,25 @@ def _entity_from_protobuf(protobuf): return _entity_from_ds_entity(ds_entity) +def _properties_of(entity): + seen = set() + + for cls in type(entity).mro(): + if not hasattr(cls, "_properties"): + continue + + for prop in cls._properties.values(): + if ( + not isinstance(prop, Property) + or isinstance(prop, ModelKey) + or prop._name in seen + ): + continue + + seen.add(prop._name) + yield prop + + def _entity_to_ds_entity(entity, set_key=True): """Convert an NDB entity to Datastore entity. @@ -662,33 +681,20 @@ def _entity_to_ds_entity(entity, set_key=True): uninitialized = [] exclude_from_indexes = [] - for cls in type(entity).mro(): - if not hasattr(cls, "_properties"): - continue - - for prop in cls._properties.values(): - if ( - not isinstance(prop, Property) - or isinstance(prop, ModelKey) - or prop._name in data - ): - continue - - if not prop._is_initialized(entity): - uninitialized.append(prop._name) + for prop in _properties_of(entity): + if not prop._is_initialized(entity): + uninitialized.append(prop._name) - value = prop._get_base_value_unwrapped_as_list(entity) - if not prop._repeated: - value = value[0] - data[prop._name] = value + names = prop._to_datastore(entity, data) - if not prop._indexed: - exclude_from_indexes.append(prop._name) + if not prop._indexed: + for name in names: + exclude_from_indexes.append(name) if uninitialized: - names = ", ".join(uninitialized) + missing = ", ".join(uninitialized) raise exceptions.BadValueError( - "Entity has uninitialized properties: {}".format(names) + "Entity has uninitialized properties: {}".format(missing) ) ds_entity = None @@ -1984,6 +1990,38 @@ def _get_for_dict(self, entity): """ return self._get_value(entity) + def _to_datastore(self, entity, data, prefix="", repeated=False): + """Helper to convert property to Datastore serializable data. + + Called to help assemble a Datastore entity prior to serialization for + storage. Subclasses (like StructuredProperty) may need to override the + default behavior. + + Args: + entity (entity.Entity): The NDB entity to convert. + data (dict): The data that will eventually be used to construct the + Datastore entity. This method works by updating ``data``. + prefix (str): Optional name prefix used for StructuredProperty (if + present, must end in ".". + repeated (bool): `True` if values should be repeated because an + ancestor node is repeated property. + + Return: + Sequence[str]: Any keys that were set on ``data`` by this method + call. + """ + value = self._get_base_value_unwrapped_as_list(entity) + if not self._repeated: + value = value[0] + + key = prefix + self._name + if repeated: + data.setdefault(key, []).append(value) + else: + data[key] = value + + return (key,) + def _validate_key(value, entity=None): """Validate a key. @@ -3861,6 +3899,46 @@ def _get_value_size(self, entity): values = [values] return len(values) + def _to_datastore(self, entity, data, prefix="", repeated=False): + """Override of :method:`StructuredProperty._to_datastore`. + + If ``legacy_data`` is ``True``, then we need to override the default + behavior to store everything in a single Datastore entity that uses + dotted attribute names, rather than nesting entities. + """ + context = context_module.get_context() + + # The easy way + if not context.legacy_data: + return super(StructuredProperty, self)._to_datastore( + entity, data, prefix=prefix, repeated=repeated + ) + + # The hard way + next_prefix = prefix + self._name + "." + next_repeated = repeated or self._repeated + keys = [] + + values = self._get_user_value(entity) + if not self._repeated: + values = (values,) + + for value in values: + if value is None: + keys.extend( + super(StructuredProperty, self)._to_datastore( + entity, data, prefix=prefix, repeated=repeated + ) + ) + continue + + for prop in _properties_of(value): + keys.extend(prop._to_datastore( + value, data, prefix=next_prefix, repeated=next_repeated + )) + + return keys + class LocalStructuredProperty(BlobProperty): """A property that contains ndb.Model value. diff --git a/tests/system/conftest.py b/tests/system/conftest.py index 8b8439fe..6abc23aa 100644 --- a/tests/system/conftest.py +++ b/tests/system/conftest.py @@ -107,5 +107,8 @@ def namespace(): @pytest.fixture def client_context(namespace): client = ndb.Client(namespace=namespace) - with client.context(cache_policy=False) as the_context: + with client.context( + cache_policy=False, + legacy_data=False, + ) as the_context: yield the_context diff --git a/tests/system/test_crud.py b/tests/system/test_crud.py index 5241ba76..5774316b 100644 --- a/tests/system/test_crud.py +++ b/tests/system/test_crud.py @@ -728,6 +728,36 @@ class SomeKind(ndb.Model): dispose_of(key._key) +def test_insert_entity_with_structured_property_legacy_data( + client_context, dispose_of, ds_client +): + class OtherKind(ndb.Model): + one = ndb.StringProperty() + two = ndb.StringProperty() + + class SomeKind(ndb.Model): + foo = ndb.IntegerProperty() + bar = ndb.StructuredProperty(OtherKind) + + with client_context.new(legacy_data=True).use(): + entity = SomeKind(foo=42, bar=OtherKind(one="hi", two="mom")) + key = entity.put() + + retrieved = key.get() + assert retrieved.foo == 42 + assert retrieved.bar.one == "hi" + assert retrieved.bar.two == "mom" + + assert isinstance(retrieved.bar, OtherKind) + + ds_entity = ds_client.get(key._key) + assert ds_entity["foo"] == 42 + assert ds_entity["bar.one"] == "hi" + assert ds_entity["bar.two"] == "mom" + + dispose_of(key._key) + + @pytest.mark.usefixtures("client_context") def test_retrieve_entity_with_legacy_structured_property(ds_entity): class OtherKind(ndb.Model): diff --git a/tests/system/test_query.py b/tests/system/test_query.py index 8f8c0836..205e6e55 100644 --- a/tests/system/test_query.py +++ b/tests/system/test_query.py @@ -628,6 +628,54 @@ def make_entities(): assert results[1].foo == 2 +def test_query_structured_property_legacy_data(client_context, dispose_of): + class OtherKind(ndb.Model): + one = ndb.StringProperty() + two = ndb.StringProperty() + three = ndb.StringProperty() + + class SomeKind(ndb.Model): + foo = ndb.IntegerProperty() + bar = ndb.StructuredProperty(OtherKind) + + @ndb.synctasklet + def make_entities(): + entity1 = SomeKind( + foo=1, bar=OtherKind(one="pish", two="posh", three="pash") + ) + entity2 = SomeKind( + foo=2, bar=OtherKind(one="pish", two="posh", three="push") + ) + entity3 = SomeKind( + foo=3, + bar=OtherKind(one="pish", two="moppish", three="pass the peas"), + ) + + keys = yield ( + entity1.put_async(), + entity2.put_async(), + entity3.put_async(), + ) + raise ndb.Return(keys) + + with client_context.new(legacy_data=True).use(): + keys = make_entities() + eventually(SomeKind.query().fetch, _length_equals(3)) + for key in keys: + dispose_of(key._key) + + query = ( + SomeKind.query() + .filter(SomeKind.bar.one == "pish", SomeKind.bar.two == "posh") + .order(SomeKind.foo) + ) + + results = query.fetch() + assert len(results) == 2 + assert results[0].foo == 1 + assert results[1].foo == 2 + + @pytest.mark.usefixtures("client_context") def test_query_legacy_structured_property(ds_entity): class OtherKind(ndb.Model): @@ -796,6 +844,67 @@ def make_entities(): assert results[1].foo == 2 +def test_query_repeated_structured_property_with_properties_legacy_data( + client_context, dispose_of +): + class OtherKind(ndb.Model): + one = ndb.StringProperty() + two = ndb.StringProperty() + three = ndb.StringProperty() + + class SomeKind(ndb.Model): + foo = ndb.IntegerProperty() + bar = ndb.StructuredProperty(OtherKind, repeated=True) + + @ndb.synctasklet + def make_entities(): + entity1 = SomeKind( + foo=1, + bar=[ + OtherKind(one="pish", two="posh", three="pash"), + OtherKind(one="bish", two="bosh", three="bash"), + ], + ) + entity2 = SomeKind( + foo=2, + bar=[ + OtherKind(one="pish", two="bosh", three="bass"), + OtherKind(one="bish", two="posh", three="pass"), + ], + ) + entity3 = SomeKind( + foo=3, + bar=[ + OtherKind(one="fish", two="fosh", three="fash"), + OtherKind(one="bish", two="bosh", three="bash"), + ], + ) + + keys = yield ( + entity1.put_async(), + entity2.put_async(), + entity3.put_async(), + ) + raise ndb.Return(keys) + + with client_context.new(legacy_data=True).use(): + keys = make_entities() + eventually(SomeKind.query().fetch, _length_equals(3)) + for key in keys: + dispose_of(key._key) + + query = ( + SomeKind.query() + .filter(SomeKind.bar.one == "pish", SomeKind.bar.two == "posh") + .order(SomeKind.foo) + ) + + results = query.fetch() + assert len(results) == 2 + assert results[0].foo == 1 + assert results[1].foo == 2 + + @pytest.mark.usefixtures("client_context") def test_query_repeated_structured_property_with_entity_twice(dispose_of): class OtherKind(ndb.Model): @@ -857,6 +966,69 @@ def make_entities(): assert results[0].foo == 1 +def test_query_repeated_structured_property_with_entity_twice_legacy_data( + client_context, dispose_of +): + class OtherKind(ndb.Model): + one = ndb.StringProperty() + two = ndb.StringProperty() + three = ndb.StringProperty() + + class SomeKind(ndb.Model): + foo = ndb.IntegerProperty() + bar = ndb.StructuredProperty(OtherKind, repeated=True) + + @ndb.synctasklet + def make_entities(): + entity1 = SomeKind( + foo=1, + bar=[ + OtherKind(one="pish", two="posh", three="pash"), + OtherKind(one="bish", two="bosh", three="bash"), + ], + ) + entity2 = SomeKind( + foo=2, + bar=[ + OtherKind(one="bish", two="bosh", three="bass"), + OtherKind(one="pish", two="posh", three="pass"), + ], + ) + entity3 = SomeKind( + foo=3, + bar=[ + OtherKind(one="pish", two="fosh", three="fash"), + OtherKind(one="bish", two="posh", three="bash"), + ], + ) + + keys = yield ( + entity1.put_async(), + entity2.put_async(), + entity3.put_async(), + ) + raise ndb.Return(keys) + + with client_context.new(legacy_data=True).use(): + keys = make_entities() + eventually(SomeKind.query().fetch, _length_equals(3)) + for key in keys: + dispose_of(key._key) + + query = ( + SomeKind.query() + .filter( + SomeKind.bar == OtherKind(one="pish", two="posh"), + SomeKind.bar == OtherKind(two="posh", three="pash"), + ) + .order(SomeKind.foo) + ) + + results = query.fetch() + assert len(results) == 1 + assert results[0].foo == 1 + + @pytest.mark.usefixtures("client_context") def test_query_repeated_structured_property_with_projection(dispose_of): class OtherKind(ndb.Model): @@ -943,6 +1115,94 @@ def sort_key(result): results[3].bar[0].three +def test_query_repeated_structured_property_with_projection_legacy_data( + client_context, dispose_of +): + class OtherKind(ndb.Model): + one = ndb.StringProperty() + two = ndb.StringProperty() + three = ndb.StringProperty() + + class SomeKind(ndb.Model): + foo = ndb.IntegerProperty() + bar = ndb.StructuredProperty(OtherKind, repeated=True) + + @ndb.synctasklet + def make_entities(): + entity1 = SomeKind( + foo=1, + bar=[ + OtherKind(one="angle", two="cankle", three="pash"), + OtherKind(one="bangle", two="dangle", three="bash"), + ], + ) + entity2 = SomeKind( + foo=2, + bar=[ + OtherKind(one="bish", two="bosh", three="bass"), + OtherKind(one="pish", two="posh", three="pass"), + ], + ) + entity3 = SomeKind( + foo=3, + bar=[ + OtherKind(one="pish", two="fosh", three="fash"), + OtherKind(one="bish", two="posh", three="bash"), + ], + ) + + keys = yield ( + entity1.put_async(), + entity2.put_async(), + entity3.put_async(), + ) + raise ndb.Return(keys) + + with client_context.new(legacy_data=True).use(): + keys = make_entities() + eventually(SomeKind.query().fetch, _length_equals(3)) + for key in keys: + dispose_of(key._key) + + query = SomeKind.query(projection=("bar.one", "bar.two")).filter( + SomeKind.foo < 2 + ) + + # This counter-intuitive result is consistent with Legacy NDB behavior and + # is a result of the odd way Datastore handles projection queries with + # array valued properties: + # + # https://cloud.google.com/datastore/docs/concepts/queries#projections_and_array-valued_properties + # + results = query.fetch() + assert len(results) == 4 + + def sort_key(result): + return (result.bar[0].one, result.bar[0].two) + + results = sorted(results, key=sort_key) + + assert results[0].bar[0].one == "angle" + assert results[0].bar[0].two == "cankle" + with pytest.raises(ndb.UnprojectedPropertyError): + results[0].bar[0].three + + assert results[1].bar[0].one == "angle" + assert results[1].bar[0].two == "dangle" + with pytest.raises(ndb.UnprojectedPropertyError): + results[1].bar[0].three + + assert results[2].bar[0].one == "bangle" + assert results[2].bar[0].two == "cankle" + with pytest.raises(ndb.UnprojectedPropertyError): + results[2].bar[0].three + + assert results[3].bar[0].one == "bangle" + assert results[3].bar[0].two == "dangle" + with pytest.raises(ndb.UnprojectedPropertyError): + results[3].bar[0].three + + @pytest.mark.usefixtures("client_context") def test_query_legacy_repeated_structured_property(ds_entity): class OtherKind(ndb.Model):