From 6e75bbabc59c5fd076e4417ccef5ccfe4b6eebcf Mon Sep 17 00:00:00 2001 From: Carlos de la Guardia Date: Tue, 23 Apr 2019 17:25:36 -0500 Subject: [PATCH 1/4] Add StructuredProperty --- src/google/cloud/ndb/model.py | 400 +++++++++++++++++++++++++++++++++- 1 file changed, 397 insertions(+), 3 deletions(-) diff --git a/src/google/cloud/ndb/model.py b/src/google/cloud/ndb/model.py index 92c4c0ef..413d6b1a 100644 --- a/src/google/cloud/ndb/model.py +++ b/src/google/cloud/ndb/model.py @@ -30,6 +30,7 @@ """ +import copy import datetime import functools import inspect @@ -109,6 +110,7 @@ _MEANING_PREDEFINED_ENTITY_USER = 20 +_MEANING_URI_COMPRESSED = "ZLIB" _MAX_STRING_LENGTH = 1500 Key = key_module.Key BlobKey = _datastore_types.BlobKey @@ -3353,11 +3355,403 @@ def _now(): return datetime.datetime.utcnow().time() +class _NestedCounter(object): + """ A recursive counter for StructuredProperty deserialization. + + Deserialization has some complicated rules to handle StructuredPropertys + that may or may not be empty. The simplest case is a leaf counter, where + the counter will return the index of the repeated value that last had this + leaf property written. When a non-leaf counter requested, this will return + the max of all its leaf values. This is due to the fact that the next index + that a full non-leaf property may be written to comes after all indices that + have part of that property written (otherwise, a partial entity would be + overwritten. + + Consider an evaluation of the following structure: + + class B(model.Model): + c = model.IntegerProperty() + d = model.IntegerProperty() + + class A(model.Model): + b = model.StructuredProperty(B) + + class Foo(model.Model): + # top-level model + a = model.StructuredProperty(A, repeated=True) + Foo(a=[A(b=None), + A(b=B(c=1)), + A(b=None), + A(b=B(c=2, d=3))]) + + This will result in a serialized structure: + + 1) a.b = None + 2) a.b.c = 1 + 3) a.b.d = None + 4) a.b = None + 5) a.b.c = 2 + 6) a.b.d = 3 + + The counter state should be the following: + + a | a.b | a.b.c | a.b.d + + 0) - - - - + 1) @1 1 - - + 2) @2 @2 2 - + 3) @2 @2 2 2 + 4) @3 @3 3 3 + 5) @4 @4 4 3 + 6) @4 @4 4 4 + + Here, @ indicates that this counter value is actually a calculated value. + It is equal to the MAX of its sub-counters. + + Counter values may get incremented multiple times while deserializing a + property. This will happen if a child counter falls behind, + for example in steps 2 and 3. + + During an increment of a parent node, all child nodes values are incremented + to match that of the parent, for example in step 4. + """ + + def __init__(self): + self._counter = 0 + self._sub_counters = collections.defaultdict(_NestedCounter) + + def get(self, parts=None): + if parts: + return self._sub_counters[parts[0]].get(parts[1:]) + if self._is_parent_node(): + return max(v.get() for v in self._sub_counters.itervalues()) + return self._counter + + def increment(self, parts=None): + if parts: + self._make_parent_node() + return self._sub_counters[parts[0]].increment(parts[1:]) + if self._is_parent_node(): + # Move all children forward + value = self.get() + 1 + self._set(value) + return value + self._counter += 1 + return self._counter + + def _set(self, value): + """Updates all descendants to a specified value.""" + if self._is_parent_node(): + for child in self._sub_counters.itervalues(): + child._set(value) + else: + self._counter = value + + def _absolute_counter(self): + # Used only for testing. + return self._counter + + def _is_parent_node(self): + return self._counter == -1 + + def _make_parent_node(self): + self._counter = -1 + + class StructuredProperty(Property): - __slots__ = () + """A Property whose value is itself an entity. - def __init__(self, *args, **kwargs): - raise NotImplementedError + The values of the sub-entity are indexed and can be queried. + See the module docstring for details. + """ + + _modelclass = None + + _attributes = ['_modelclass'] + Property._attributes + + def __init__(self, modelclass, name=None, **kwds): + super(StructuredProperty, self).__init__(name=name, **kwds) + if self._repeated: + if modelclass._has_repeated: + raise TypeError('This StructuredProperty cannot use repeated=True ' + 'because its model class (%s) contains repeated ' + 'properties (directly or indirectly).' % + modelclass.__name__) + self._modelclass = modelclass + + def _get_value(self, entity): + """Override _get_value() to *not* raise UnprojectedPropertyError.""" + value = self._get_user_value(entity) + if value is None and entity._projection: + # Invoke super _get_value() to raise the proper exception. + return super(StructuredProperty, self)._get_value(entity) + return value + + def _get_for_dict(self, entity): + value = self._get_value(entity) + if self._repeated: + value = [v._to_dict() for v in value] + elif value is not None: + value = value._to_dict() + return value + + def __getattr__(self, attrname): + """Dynamically get a subproperty.""" + # Optimistically try to use the dict key. + prop = self._modelclass._properties.get(attrname) + # We're done if we have a hit and _code_name matches. + if prop is None or prop._code_name != attrname: + # Otherwise, use linear search looking for a matching _code_name. + for prop in self._modelclass._properties.values(): + if prop._code_name == attrname: + break + else: + # This is executed when we never execute the above break. + prop = None + if prop is None: + raise AttributeError('Model subclass %s has no attribute %s' % + (self._modelclass.__name__, attrname)) + prop_copy = copy.copy(prop) + prop_copy._name = self._name + '.' + prop_copy._name + # Cache the outcome, so subsequent requests for the same attribute + # name will get the copied property directly rather than going + # through the above motions all over again. + setattr(self, attrname, prop_copy) + return prop_copy + + def _comparison(self, op, value): + if op != '=': + raise exceptions.BadFilterError( + 'StructuredProperty filter can only use ==') + if not self._indexed: + raise exceptions.BadFilterError( + 'Cannot query for unindexed StructuredProperty %s' % self._name) + # Import late to avoid circular imports. + from .query import ConjunctionNode, PostFilterNode + from .query import RepeatedStructuredPropertyPredicate + if value is None: + from .query import FilterNode # Import late to avoid circular imports. + return FilterNode(self._name, op, value) + value = self._do_validate(value) + value = self._call_to_base_type(value) + filters = [] + match_keys = [] + # TODO: Why not just iterate over value._values? + for prop in self._modelclass._properties.itervalues(): + vals = prop._get_base_value_unwrapped_as_list(value) + if prop._repeated: + if vals: + raise exceptions.BadFilterError( + 'Cannot query for non-empty repeated property %s' % prop._name) + continue + assert isinstance(vals, list) and len(vals) == 1, repr(vals) + val = vals[0] + if val is not None: + altprop = getattr(self, prop._code_name) + filt = altprop._comparison(op, val) + filters.append(filt) + match_keys.append(altprop._name) + if not filters: + raise exceptions.BadFilterError( + 'StructuredProperty filter without any values') + if len(filters) == 1: + return filters[0] + if self._repeated: + pb = value._to_pb(allow_partial=True) + pred = RepeatedStructuredPropertyPredicate(match_keys, pb, + self._name + '.') + filters.append(PostFilterNode(pred)) + return ConjunctionNode(*filters) + + def _IN(self, value): + if not isinstance(value, (list, tuple, set, frozenset)): + raise exceptions.BadArgumentError( + 'Expected list, tuple or set, got %r' % (value,)) + from .query import DisjunctionNode, FalseNode + # Expand to a series of == filters. + filters = [self._comparison('=', val) for val in value] + if not filters: + # DisjunctionNode doesn't like an empty list of filters. + # Running the query will still fail, but this matches the + # behavior of IN for regular properties. + return FalseNode() + else: + return DisjunctionNode(*filters) + IN = _IN + + def _validate(self, value): + if isinstance(value, dict): + # A dict is assumed to be the result of a _to_dict() call. + return self._modelclass(**value) + if not isinstance(value, self._modelclass): + raise exceptions.BadValueError('Expected %s instance, got %r' % + (self._modelclass.__name__, value)) + + def _has_value(self, entity, rest=None): + # rest: optional list of attribute names to check in addition. + # Basically, prop._has_value(self, ent, ['x', 'y']) is similar to + # (prop._has_value(ent) and + # prop.x._has_value(ent.x) and + # prop.x.y._has_value(ent.x.y)) + # assuming prop.x and prop.x.y exist. + # NOTE: This is not particularly efficient if len(rest) > 1, + # but that seems a rare case, so for now I don't care. + ok = super(StructuredProperty, self)._has_value(entity) + if ok and rest: + lst = self._get_base_value_unwrapped_as_list(entity) + if len(lst) != 1: + raise RuntimeError('Failed to retrieve sub-entity of StructuredProperty' + ' %s' % self._name) + subent = lst[0] + if subent is None: + return True + subprop = subent._properties.get(rest[0]) + if subprop is None: + ok = False + else: + ok = subprop._has_value(subent, rest[1:]) + return ok + + def _serialize(self, entity, pb, prefix='', parent_repeated=False, + projection=None): + # entity -> pb; pb is an EntityProto message + values = self._get_base_value_unwrapped_as_list(entity) + for value in values: + if value is not None: + # TODO: Avoid re-sorting for repeated values. + for unused_name, prop in sorted(value._properties.iteritems()): + prop._serialize(value, pb, prefix + self._name + '.', + self._repeated or parent_repeated, + projection=projection) + else: + # Serialize a single None + super(StructuredProperty, self)._serialize( + entity, pb, prefix=prefix, parent_repeated=parent_repeated, + projection=projection) + + def _deserialize(self, entity, p, depth=1): + if not self._repeated: + subentity = self._retrieve_value(entity) + if subentity is None: + subentity = self._modelclass() + self._store_value(entity, _BaseValue(subentity)) + cls = self._modelclass + if isinstance(subentity, _BaseValue): + # NOTE: It may not be a _BaseValue when we're deserializing a + # repeated structured property. + subentity = subentity.b_val + if not isinstance(subentity, cls): + raise RuntimeError('Cannot deserialize StructuredProperty %s; value ' + 'retrieved not a %s instance %r' % + (self._name, cls.__name__, subentity)) + # _GenericProperty tries to keep compressed values as unindexed, but + # won't override a set argument. We need to force it at this level. + # TODO(pcostello): Remove this hack by passing indexed to _deserialize. + # This cannot happen until we version the API. + indexed = p.meaning_uri() != _MEANING_URI_COMPRESSED + prop = subentity._get_property_for(p, depth=depth, indexed=indexed) + if prop is None: + # Special case: kill subentity after all. + self._store_value(entity, None) + return + prop._deserialize(subentity, p, depth + 1) + return + + # The repeated case is more complicated. + # TODO: Prove we won't get here for orphans. + name = p.name() + parts = name.split('.') + if len(parts) <= depth: + raise RuntimeError('StructuredProperty %s expected to find properties ' + 'separated by periods at a depth of %i; received %r' % + (self._name, depth, parts)) + next = parts[depth] + rest = parts[depth + 1:] + prop = self._modelclass._properties.get(next) + prop_is_fake = False + if prop is None: + # Synthesize a fake property. (We can't use Model._fake_property() + # because we need the property before we can determine the subentity.) + if rest: + # TODO: Handle this case, too. + return + # TODO: Figure out the value for indexed. Unfortunately we'd + # need this passed in from _from_pb(), which would mean a + # signature change for _deserialize(), which might break valid + # end-user code that overrides it. + compressed = p.meaning_uri() == _MEANING_URI_COMPRESSED + prop = GenericProperty(next, compressed=compressed) + prop._code_name = next + prop_is_fake = True + + # Find the first subentity that doesn't have a value for this + # property yet. + if not hasattr(entity, '_subentity_counter'): + entity._subentity_counter = _NestedCounter() + counter = entity._subentity_counter + counter_path = parts[depth - 1:] + next_index = counter.get(counter_path) + subentity = None + if self._has_value(entity): + # If an entire subentity has been set to None, we have to loop + # to advance until we find the next partial entity. + while next_index < self._get_value_size(entity): + subentity = self._get_base_value_at_index(entity, next_index) + if not isinstance(subentity, self._modelclass): + raise TypeError('sub-entities must be instances ' + 'of their Model class.') + if not prop._has_value(subentity, rest): + break + next_index = counter.increment(counter_path) + else: + subentity = None + # The current property is going to be populated, so advance the counter. + counter.increment(counter_path) + if not subentity: + # We didn't find one. Add a new one to the underlying list of + # values. + subentity = self._modelclass() + values = self._retrieve_value(entity, self._default) + if values is None: + self._store_value(entity, []) + values = self._retrieve_value(entity, self._default) + values.append(_BaseValue(subentity)) + if prop_is_fake: + # Add the synthetic property to the subentity's _properties + # dict, so that it will be correctly deserialized. + # (See Model._fake_property() for comparison.) + subentity._clone_properties() + subentity._properties[prop._name] = prop + prop._deserialize(subentity, p, depth + 1) + + def _prepare_for_put(self, entity): + values = self._get_base_value_unwrapped_as_list(entity) + for value in values: + if value is not None: + value._prepare_for_put() + + def _check_property(self, rest=None, require_indexed=True): + """Override for Property._check_property(). + Raises: + InvalidPropertyError if no subproperty is specified or if something + is wrong with the subproperty. + """ + if not rest: + raise InvalidPropertyError( + 'Structured property %s requires a subproperty' % self._name) + self._modelclass._check_properties([rest], require_indexed=require_indexed) + + def _get_base_value_at_index(self, entity, index): + assert self._repeated + value = self._retrieve_value(entity, self._default) + value[index] = self._opt_call_to_base_type(value[index]) + return value[index].b_val + + def _get_value_size(self, entity): + values = self._retrieve_value(entity, self._default) + if values is None: + return 0 + return len(values) class LocalStructuredProperty(BlobProperty): From 28861e0ac7aa82757e7af2a31ac2c2182c4de058 Mon Sep 17 00:00:00 2001 From: Carlos de la Guardia Date: Fri, 17 May 2019 03:54:44 -0500 Subject: [PATCH 2/4] wip: model properties (structured, generic and computed) --- src/google/cloud/ndb/model.py | 375 ++++++++++----------------- tests/unit/test_model.py | 464 +++++++++++++++++++++++++++++++++- 2 files changed, 590 insertions(+), 249 deletions(-) diff --git a/src/google/cloud/ndb/model.py b/src/google/cloud/ndb/model.py index 413d6b1a..03b1ad73 100644 --- a/src/google/cloud/ndb/model.py +++ b/src/google/cloud/ndb/model.py @@ -3355,122 +3355,16 @@ def _now(): return datetime.datetime.utcnow().time() -class _NestedCounter(object): - """ A recursive counter for StructuredProperty deserialization. - - Deserialization has some complicated rules to handle StructuredPropertys - that may or may not be empty. The simplest case is a leaf counter, where - the counter will return the index of the repeated value that last had this - leaf property written. When a non-leaf counter requested, this will return - the max of all its leaf values. This is due to the fact that the next index - that a full non-leaf property may be written to comes after all indices that - have part of that property written (otherwise, a partial entity would be - overwritten. - - Consider an evaluation of the following structure: - - class B(model.Model): - c = model.IntegerProperty() - d = model.IntegerProperty() - - class A(model.Model): - b = model.StructuredProperty(B) - - class Foo(model.Model): - # top-level model - a = model.StructuredProperty(A, repeated=True) - Foo(a=[A(b=None), - A(b=B(c=1)), - A(b=None), - A(b=B(c=2, d=3))]) - - This will result in a serialized structure: - - 1) a.b = None - 2) a.b.c = 1 - 3) a.b.d = None - 4) a.b = None - 5) a.b.c = 2 - 6) a.b.d = 3 - - The counter state should be the following: - - a | a.b | a.b.c | a.b.d - - 0) - - - - - 1) @1 1 - - - 2) @2 @2 2 - - 3) @2 @2 2 2 - 4) @3 @3 3 3 - 5) @4 @4 4 3 - 6) @4 @4 4 4 - - Here, @ indicates that this counter value is actually a calculated value. - It is equal to the MAX of its sub-counters. - - Counter values may get incremented multiple times while deserializing a - property. This will happen if a child counter falls behind, - for example in steps 2 and 3. - - During an increment of a parent node, all child nodes values are incremented - to match that of the parent, for example in step 4. - """ - - def __init__(self): - self._counter = 0 - self._sub_counters = collections.defaultdict(_NestedCounter) - - def get(self, parts=None): - if parts: - return self._sub_counters[parts[0]].get(parts[1:]) - if self._is_parent_node(): - return max(v.get() for v in self._sub_counters.itervalues()) - return self._counter - - def increment(self, parts=None): - if parts: - self._make_parent_node() - return self._sub_counters[parts[0]].increment(parts[1:]) - if self._is_parent_node(): - # Move all children forward - value = self.get() + 1 - self._set(value) - return value - self._counter += 1 - return self._counter - - def _set(self, value): - """Updates all descendants to a specified value.""" - if self._is_parent_node(): - for child in self._sub_counters.itervalues(): - child._set(value) - else: - self._counter = value - - def _absolute_counter(self): - # Used only for testing. - return self._counter - - def _is_parent_node(self): - return self._counter == -1 - - def _make_parent_node(self): - self._counter = -1 - - class StructuredProperty(Property): """A Property whose value is itself an entity. The values of the sub-entity are indexed and can be queried. - See the module docstring for details. """ _modelclass = None - _attributes = ['_modelclass'] + Property._attributes - - def __init__(self, modelclass, name=None, **kwds): - super(StructuredProperty, self).__init__(name=name, **kwds) + def __init__(self, modelclass, name=None, **kwargs): + super(StructuredProperty, self).__init__(name=name, **kwargs) if self._repeated: if modelclass._has_repeated: raise TypeError('This StructuredProperty cannot use repeated=True ' @@ -3500,14 +3394,6 @@ def __getattr__(self, attrname): # Optimistically try to use the dict key. prop = self._modelclass._properties.get(attrname) # We're done if we have a hit and _code_name matches. - if prop is None or prop._code_name != attrname: - # Otherwise, use linear search looking for a matching _code_name. - for prop in self._modelclass._properties.values(): - if prop._code_name == attrname: - break - else: - # This is executed when we never execute the above break. - prop = None if prop is None: raise AttributeError('Model subclass %s has no attribute %s' % (self._modelclass.__name__, attrname)) @@ -3536,8 +3422,7 @@ def _comparison(self, op, value): value = self._call_to_base_type(value) filters = [] match_keys = [] - # TODO: Why not just iterate over value._values? - for prop in self._modelclass._properties.itervalues(): + for prop in self._modelclass._properties.values(): vals = prop._get_base_value_unwrapped_as_list(value) if prop._repeated: if vals: @@ -3557,10 +3442,11 @@ def _comparison(self, op, value): if len(filters) == 1: return filters[0] if self._repeated: - pb = value._to_pb(allow_partial=True) - pred = RepeatedStructuredPropertyPredicate(match_keys, pb, - self._name + '.') - filters.append(PostFilterNode(pred)) + raise NotImplementedError("This depends on code not yet ported.") + # pb = value._to_pb(allow_partial=True) + # pred = RepeatedStructuredPropertyPredicate(match_keys, pb, + # self._name + '.') + # filters.append(PostFilterNode(pred)) return ConjunctionNode(*filters) def _IN(self, value): @@ -3584,8 +3470,8 @@ def _validate(self, value): # A dict is assumed to be the result of a _to_dict() call. return self._modelclass(**value) if not isinstance(value, self._modelclass): - raise exceptions.BadValueError('Expected %s instance, got %r' % - (self._modelclass.__name__, value)) + raise exceptions.BadValueError('Expected %s instance, got %s' % + (self._modelclass.__name__, value.__class__)) def _has_value(self, entity, rest=None): # rest: optional list of attribute names to check in addition. @@ -3612,118 +3498,6 @@ def _has_value(self, entity, rest=None): ok = subprop._has_value(subent, rest[1:]) return ok - def _serialize(self, entity, pb, prefix='', parent_repeated=False, - projection=None): - # entity -> pb; pb is an EntityProto message - values = self._get_base_value_unwrapped_as_list(entity) - for value in values: - if value is not None: - # TODO: Avoid re-sorting for repeated values. - for unused_name, prop in sorted(value._properties.iteritems()): - prop._serialize(value, pb, prefix + self._name + '.', - self._repeated or parent_repeated, - projection=projection) - else: - # Serialize a single None - super(StructuredProperty, self)._serialize( - entity, pb, prefix=prefix, parent_repeated=parent_repeated, - projection=projection) - - def _deserialize(self, entity, p, depth=1): - if not self._repeated: - subentity = self._retrieve_value(entity) - if subentity is None: - subentity = self._modelclass() - self._store_value(entity, _BaseValue(subentity)) - cls = self._modelclass - if isinstance(subentity, _BaseValue): - # NOTE: It may not be a _BaseValue when we're deserializing a - # repeated structured property. - subentity = subentity.b_val - if not isinstance(subentity, cls): - raise RuntimeError('Cannot deserialize StructuredProperty %s; value ' - 'retrieved not a %s instance %r' % - (self._name, cls.__name__, subentity)) - # _GenericProperty tries to keep compressed values as unindexed, but - # won't override a set argument. We need to force it at this level. - # TODO(pcostello): Remove this hack by passing indexed to _deserialize. - # This cannot happen until we version the API. - indexed = p.meaning_uri() != _MEANING_URI_COMPRESSED - prop = subentity._get_property_for(p, depth=depth, indexed=indexed) - if prop is None: - # Special case: kill subentity after all. - self._store_value(entity, None) - return - prop._deserialize(subentity, p, depth + 1) - return - - # The repeated case is more complicated. - # TODO: Prove we won't get here for orphans. - name = p.name() - parts = name.split('.') - if len(parts) <= depth: - raise RuntimeError('StructuredProperty %s expected to find properties ' - 'separated by periods at a depth of %i; received %r' % - (self._name, depth, parts)) - next = parts[depth] - rest = parts[depth + 1:] - prop = self._modelclass._properties.get(next) - prop_is_fake = False - if prop is None: - # Synthesize a fake property. (We can't use Model._fake_property() - # because we need the property before we can determine the subentity.) - if rest: - # TODO: Handle this case, too. - return - # TODO: Figure out the value for indexed. Unfortunately we'd - # need this passed in from _from_pb(), which would mean a - # signature change for _deserialize(), which might break valid - # end-user code that overrides it. - compressed = p.meaning_uri() == _MEANING_URI_COMPRESSED - prop = GenericProperty(next, compressed=compressed) - prop._code_name = next - prop_is_fake = True - - # Find the first subentity that doesn't have a value for this - # property yet. - if not hasattr(entity, '_subentity_counter'): - entity._subentity_counter = _NestedCounter() - counter = entity._subentity_counter - counter_path = parts[depth - 1:] - next_index = counter.get(counter_path) - subentity = None - if self._has_value(entity): - # If an entire subentity has been set to None, we have to loop - # to advance until we find the next partial entity. - while next_index < self._get_value_size(entity): - subentity = self._get_base_value_at_index(entity, next_index) - if not isinstance(subentity, self._modelclass): - raise TypeError('sub-entities must be instances ' - 'of their Model class.') - if not prop._has_value(subentity, rest): - break - next_index = counter.increment(counter_path) - else: - subentity = None - # The current property is going to be populated, so advance the counter. - counter.increment(counter_path) - if not subentity: - # We didn't find one. Add a new one to the underlying list of - # values. - subentity = self._modelclass() - values = self._retrieve_value(entity, self._default) - if values is None: - self._store_value(entity, []) - values = self._retrieve_value(entity, self._default) - values.append(_BaseValue(subentity)) - if prop_is_fake: - # Add the synthetic property to the subentity's _properties - # dict, so that it will be correctly deserialized. - # (See Model._fake_property() for comparison.) - subentity._clone_properties() - subentity._properties[prop._name] = prop - prop._deserialize(subentity, p, depth + 1) - def _prepare_for_put(self, entity): values = self._get_base_value_unwrapped_as_list(entity) for value in values: @@ -3751,6 +3525,8 @@ def _get_value_size(self, entity): values = self._retrieve_value(entity, self._default) if values is None: return 0 + if not isinstance(values, list): + values = [values] return len(values) @@ -3845,17 +3621,126 @@ def _from_base_type(self, value): class GenericProperty(Property): - __slots__ = () + """A Property whose value can be (almost) any basic type. + This is mainly used for Expando and for orphans (values present in + Cloud Datastore but not represented in the Model subclass) but can + also be used explicitly for properties with dynamically-typed + values. + + This supports compressed=True, which is only effective for str + values (not for unicode), and implies indexed=False. + """ - def __init__(self, *args, **kwargs): - raise NotImplementedError + _compressed = False + + def __init__(self, name=None, compressed=False, **kwargs): + if compressed: # Compressed implies unindexed. + kwargs.setdefault('indexed', False) + super(GenericProperty, self).__init__(name=name, **kwargs) + self._compressed = compressed + if compressed and self._indexed: + # TODO: Allow this, but only allow == and IN comparisons? + raise NotImplementedError('GenericProperty %s cannot be compressed and ' + 'indexed at the same time.' % self._name) + + def _to_base_type(self, value): + if self._compressed and isinstance(value, bytes): + return _CompressedValue(zlib.compress(value)) + + def _from_base_type(self, value): + if isinstance(value, _CompressedValue): + return zlib.decompress(value.z_val) + + def _validate(self, value): + if self._indexed: + if isinstance(value, bytes) and len(value) > _MAX_STRING_LENGTH: + raise exceptions.BadValueError( + 'Indexed value %s must be at most %d bytes' % + (self._name, _MAX_STRING_LENGTH)) + + def _db_get_value(self, v, unused_p): + """Helper for :meth:`_deserialize`. + + Raises: + NotImplementedError: Always. This method is deprecated. + """ + raise exceptions.NoLongerImplementedError() + + def _db_set_value(self, v, p, value): + """Helper for :meth:`_deserialize`. + + Raises: + NotImplementedError: Always. This method is deprecated. + """ + raise exceptions.NoLongerImplementedError() class ComputedProperty(GenericProperty): - __slots__ = () + """A Property whose value is determined by a user-supplied function. + Computed properties cannot be set directly, but are instead generated by a + function when required. They are useful to provide fields in Cloud Datastore + that can be used for filtering or sorting without having to manually set the + value in code - for example, sorting on the length of a BlobProperty, or + using an equality filter to check if another field is not empty. + ComputedProperty can be declared as a regular property, passing a function as + the first argument, or it can be used as a decorator for the function that + does the calculation. + + Example: + + >>> class DatastoreFile(Model): + ... name = StringProperty() + ... name_lower = ComputedProperty(lambda self: self.name.lower()) + ... + ... data = BlobProperty() + ... + ... @ComputedProperty + ... def size(self): + ... return len(self.data) + ... + ... def _compute_hash(self): + ... return hashlib.sha1(self.data).hexdigest() + ... hash = ComputedProperty(_compute_hash, name='sha1') + """ - def __init__(self, *args, **kwargs): - raise NotImplementedError + def __init__(self, func, name=None, indexed=None, + repeated=None, verbose_name=None): + """Constructor. + Args: + + func: A function that takes one argument, the model instance, and returns + a calculated value. + """ + super(ComputedProperty, self).__init__(name=name, indexed=indexed, + repeated=repeated, + verbose_name=verbose_name) + self._func = func + + def _set_value(self, entity, value): + raise ComputedPropertyError("Cannot assign to a ComputedProperty") + + def _delete_value(self, entity): + raise ComputedPropertyError("Cannot delete a ComputedProperty") + + def _get_value(self, entity): + # About projections and computed properties: if the computed + # property itself is in the projection, don't recompute it; this + # prevents raising UnprojectedPropertyError if one of the + # dependents is not in the projection. However, if the computed + # property is not in the projection, compute it normally -- its + # dependents may all be in the projection, and it may be useful to + # access the computed value without having it in the projection. + # In this case, if any of the dependents is not in the projection, + # accessing it in the computation function will raise + # UnprojectedPropertyError which will just bubble up. + if entity._projection and self._name in entity._projection: + return super(ComputedProperty, self)._get_value(entity) + value = self._func(entity) + self._store_value(entity, value) + return value + + def _prepare_for_put(self, entity): + self._get_value(entity) # For its side effects. class MetaModel(type): @@ -4428,14 +4313,14 @@ def _validate_key(key): return key @classmethod - def _gql(cls, query_string, *args, **kwds): + def _gql(cls, query_string, *args, **kwargs): """Run a GQL query using this model as the FROM entity. Args: query_string (str): The WHERE part of a GQL query (including the WHERE kwyword). args: if present, used to call bind() on the query. - kwds: if present, used to call bind() on the query. + kwargs: if present, used to call bind() on the query. Returns: :class:query.Query: A query instance. @@ -4445,7 +4330,7 @@ def _gql(cls, query_string, *args, **kwds): return query.gql( "SELECT * FROM {} {}".format( - cls._class_name(), query_string, *args, *kwds + cls._class_name(), query_string, *args, *kwargs ) ) diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 188e63a0..b823dd32 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -2537,8 +2537,342 @@ def test__from_base_type(self): class TestStructuredProperty: @staticmethod def test_constructor(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + assert prop._modelclass == Mine + + @staticmethod + def test_constructor_with_repeated(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine, repeated=True) + assert prop._modelclass == Mine + + @staticmethod + def test_constructor_with_repeated_prop(): + class Mine(model.Model): + foo = model.StringProperty(repeated=True) + + with pytest.raises(TypeError): + model.StructuredProperty(Mine, repeated=True) + + @staticmethod + def test__validate(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + instance = Mine() + assert prop._validate(instance) is None + + @staticmethod + def test__validate_with_dict(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + assert isinstance(prop._validate({}), Mine) + + @staticmethod + def test__validate_invalid(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + with pytest.raises(exceptions.BadValueError): + prop._validate(None) + + @staticmethod + def test__get_value(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine() + minetoo = MineToo() + minetoo.bar = mine + assert MineToo.bar._get_value(minetoo) == mine + + @staticmethod + def test__get_value_unprojected(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + minetoo = MineToo(projection=("bar.foo",)) + with pytest.raises(model.UnprojectedPropertyError): + MineToo.bar._get_value(minetoo) + + @staticmethod + def test__get_for_dict(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo() + minetoo.bar = mine + assert MineToo.bar._get_for_dict(minetoo) == {"foo": "Foo"} + + @staticmethod + def test__get_for_dict_repeated(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine, repeated=True) + + mine = Mine(foo="Foo") + minetoo = MineToo() + minetoo.bar = [mine, mine] + assert MineToo.bar._get_for_dict(minetoo) == [{"foo": "Foo"}, {"foo": "Foo"}] + + @staticmethod + def test__get_for_dict_no_value(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo() + minetoo.bar = None + assert MineToo.bar._get_for_dict(minetoo) is None + + @staticmethod + def test___getattr__(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + assert isinstance(prop.foo, model.StringProperty) + assert prop.foo._name == "bar.foo" + + @staticmethod + def test___getattr___bad_prop(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + with pytest.raises(AttributeError): + prop.baz + + @staticmethod + def test__comparison_eq(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + mine = Mine(foo="baz") + assert prop._comparison("=", mine) == query_module.FilterNode("bar.foo", "=", "baz") + + @staticmethod + def test__comparison_other(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + mine = Mine(foo="baz") + with pytest.raises(exceptions.BadFilterError): + prop._comparison(">", mine) + + @staticmethod + def test__comparison_not_indexed(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine, indexed=False) + mine = Mine(foo="baz") + with pytest.raises(exceptions.BadFilterError): + prop._comparison("=", mine) + + @staticmethod + def test__comparison_value_none(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + assert prop._comparison("=", None) == query_module.FilterNode("bar", "=", None) + + @staticmethod + def test__comparison_repeated(): + class Mine(model.Model): + foo = model.StringProperty(repeated=True) + bar = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "baz" + mine = Mine(bar="x") + assert prop._comparison("=", mine) == query_module.FilterNode("baz.bar", "=", "x") + + @staticmethod + def test__comparison_repeated_no_filters(): + class Mine(model.Model): + foo = model.StringProperty(repeated=True) + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + mine = Mine(foo=[]) + with pytest.raises(exceptions.BadFilterError): + prop._comparison("=", mine) + + @staticmethod + def test__comparison_repeated_non_empty(): + class Mine(model.Model): + foo = model.StringProperty(repeated=True) + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + mine = Mine(foo=["baz"]) + with pytest.raises(exceptions.BadFilterError): + prop._comparison("=", mine) + + @staticmethod + def test__comparison_multiple(): + class Mine(model.Model): + foo = model.StringProperty() + bar = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "baz" + mine = Mine(foo="x", bar="y") + assert prop._comparison("=", mine) == query_module.AND( + query_module.FilterNode("baz.bar", "=", "y"), + query_module.FilterNode("baz.foo", "=", "x") + ) + + @staticmethod + def test__comparison_repeated_structured(): + class Mine(model.Model): + foo = model.StringProperty() + bar = model.StringProperty() + + prop = model.StructuredProperty(Mine, repeated=True) + prop._name = "bar" + mine = Mine(foo="x", bar="y") with pytest.raises(NotImplementedError): - model.StructuredProperty() + prop._comparison("=", mine) + + @staticmethod + def test_IN(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "baz" + mine = Mine(foo="x") + minetoo = Mine(foo="y") + assert prop.IN([mine, minetoo]) == query_module.OR( + query_module.FilterNode("baz.foo", "=", "x"), + query_module.FilterNode("baz.foo", "=", "y"), + ) + + @staticmethod + def test_IN_no_value(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "baz" + mine = Mine(foo="x") + minetoo = Mine(foo="y") + assert prop.IN([]) == query_module.FalseNode() + + @staticmethod + def test_IN_bad_value(): + class Mine(model.Model): + foo = model.StringProperty() + + prop = model.StructuredProperty(Mine) + prop._name = "baz" + with pytest.raises(exceptions.BadArgumentError): + prop.IN(None) + + @staticmethod + def test__has_value(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=mine) + assert MineToo.bar._has_value(minetoo) is True + + @staticmethod + def test__has_value_with_rest(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=mine) + assert MineToo.bar._has_value(minetoo, rest=["foo"]) is True + + @staticmethod + def test__has_value_with_rest_subprop_none(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=mine) + assert MineToo.bar._has_value(minetoo, rest=[None]) is False + + @staticmethod + def test__get_base_value_at_index(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine, repeated=True) + + mine = Mine(foo="Foo") + mine2 = Mine(foo="Fa") + minetoo = MineToo(bar=[mine, mine2]) + assert MineToo.bar._get_base_value_at_index(minetoo, 1) == mine2 + + @staticmethod + def test__get_value_size(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=mine) + assert MineToo.bar._get_value_size(minetoo) == 1 + + @staticmethod + def test__get_value_size_none(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=None) + assert MineToo.bar._get_value_size(minetoo) == 0 class TestLocalStructuredProperty: @@ -2646,15 +2980,137 @@ class Simple(model.Model): class TestGenericProperty: @staticmethod def test_constructor(): + prop = model.GenericProperty(name="generic") + assert prop._name == "generic" + + @staticmethod + def test_constructor_compressed(): + prop = model.GenericProperty(name="generic", compressed=True) + assert prop._compressed is True + + @staticmethod + def test_constructor_compressed_and_indexed(): with pytest.raises(NotImplementedError): - model.GenericProperty() + model.GenericProperty(name="generic", compressed=True, indexed=True) + + @staticmethod + def test__db_get_value(): + prop = model.GenericProperty() + + with pytest.raises(exceptions.NoLongerImplementedError): + prop._db_get_value(None, None) + + @staticmethod + def test__db_set_value(): + prop = model.GenericProperty() + + with pytest.raises(exceptions.NoLongerImplementedError): + prop._db_set_value(None, None, None) + + @staticmethod + def test__to_base_type(): + prop = model.GenericProperty(name="generic", compressed=True) + value = b"abc" * 10 + converted = prop._to_base_type(value) + + assert isinstance(converted, model._CompressedValue) + assert converted.z_val == zlib.compress(value) + + @staticmethod + def test__to_base_type_no_convert(): + prop = model.GenericProperty(name="generic") + value = b"abc" * 10 + converted = prop._to_base_type(value) + assert converted is None + + @staticmethod + def test__from_base_type(): + prop = model.GenericProperty(name="generic") + original = b"abc" * 10 + z_val = zlib.compress(original) + value = model._CompressedValue(z_val) + converted = prop._from_base_type(value) + + assert converted == original + + @staticmethod + def test__from_base_type_no_convert(): + prop = model.GenericProperty(name="generic") + converted = prop._from_base_type(b"abc") + assert converted is None + + @staticmethod + def test__validate(): + prop = model.GenericProperty(name="generic") + assert prop._validate(b"abc") is None + + @staticmethod + def test__validate_indexed(): + prop = model.GenericProperty(name="generic", indexed=True) + assert prop._validate(42) is None + + @staticmethod + def test__validate_indexed_bytes(): + prop = model.GenericProperty(name="generic", indexed=True) + assert prop._validate(b"abc") is None + + @staticmethod + def test__validate_indexed_unicode(): + prop = model.GenericProperty(name="generic", indexed=True) + assert prop._validate(u"abc") is None + + @staticmethod + def test__validate_indexed_bad_length(): + prop = model.GenericProperty(name="generic", indexed=True) + with pytest.raises(exceptions.BadValueError): + prop._validate(b"ab" * model._MAX_STRING_LENGTH) class TestComputedProperty: @staticmethod def test_constructor(): - with pytest.raises(NotImplementedError): - model.ComputedProperty() + def lower_name(self): + return self.lower() # pragma: NO COVER + + prop = model.ComputedProperty(lower_name) + assert prop._func == lower_name + + @staticmethod + def test__set_value(): + prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER + with pytest.raises(model.ComputedPropertyError): + prop._set_value(None, None) + + @staticmethod + def test__delete_value(): + prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER + with pytest.raises(model.ComputedPropertyError): + prop._delete_value(None) + + @staticmethod + def test__get_value(): + prop = model.ComputedProperty(lambda self: 42) + entity = unittest.mock.Mock( + _projection=None, _values={}, spec=("_projection") + ) + assert prop._get_value(entity) == 42 + + @staticmethod + def test__get_value_with_projection(): + prop = model.ComputedProperty(lambda self: 42, name="computed") # pragma: NO COVER + entity = unittest.mock.Mock( + _projection=['computed'], _values={"computed": 84}, spec=("_projection", "_values") + ) + assert prop._get_value(entity) == 84 + + @staticmethod + def test__get_value_empty_projection(): + prop = model.ComputedProperty(lambda self: 42) + entity = unittest.mock.Mock( + _projection=None, _values={}, spec=("_projection") + ) + prop._prepare_for_put(entity) + assert entity._values == {prop._name: 42} class TestMetaModel: From 95824947fcede0bdbeb0673579eeabffcfc83ec2 Mon Sep 17 00:00:00 2001 From: Carlos de la Guardia Date: Wed, 22 May 2019 00:48:08 -0500 Subject: [PATCH 3/4] review fixes and 100% coverage --- src/google/cloud/ndb/model.py | 146 +++++++++++++++++++--------------- tests/unit/test_model.py | 91 +++++++++++++++++---- 2 files changed, 155 insertions(+), 82 deletions(-) diff --git a/src/google/cloud/ndb/model.py b/src/google/cloud/ndb/model.py index 03b1ad73..414c9d69 100644 --- a/src/google/cloud/ndb/model.py +++ b/src/google/cloud/ndb/model.py @@ -47,6 +47,7 @@ from google.cloud.ndb import exceptions from google.cloud.ndb import key as key_module from google.cloud.ndb import _options +from google.cloud.ndb import query as query_module from google.cloud.ndb import _transaction from google.cloud.ndb import tasklets @@ -3367,10 +3368,12 @@ def __init__(self, modelclass, name=None, **kwargs): super(StructuredProperty, self).__init__(name=name, **kwargs) if self._repeated: if modelclass._has_repeated: - raise TypeError('This StructuredProperty cannot use repeated=True ' - 'because its model class (%s) contains repeated ' - 'properties (directly or indirectly).' % - modelclass.__name__) + raise TypeError( + "This StructuredProperty cannot use repeated=True " + "because its model class (%s) contains repeated " + "properties (directly or indirectly)." + % modelclass.__name__ + ) self._modelclass = modelclass def _get_value(self, entity): @@ -3393,12 +3396,13 @@ def __getattr__(self, attrname): """Dynamically get a subproperty.""" # Optimistically try to use the dict key. prop = self._modelclass._properties.get(attrname) - # We're done if we have a hit and _code_name matches. if prop is None: - raise AttributeError('Model subclass %s has no attribute %s' % - (self._modelclass.__name__, attrname)) + raise AttributeError( + "Model subclass %s has no attribute %s" + % (self._modelclass.__name__, attrname) + ) prop_copy = copy.copy(prop) - prop_copy._name = self._name + '.' + prop_copy._name + prop_copy._name = self._name + "." + prop_copy._name # Cache the outcome, so subsequent requests for the same attribute # name will get the copied property directly rather than going # through the above motions all over again. @@ -3406,17 +3410,23 @@ def __getattr__(self, attrname): return prop_copy def _comparison(self, op, value): - if op != '=': + if op != query_module._EQ_OP: raise exceptions.BadFilterError( - 'StructuredProperty filter can only use ==') + "StructuredProperty filter can only use ==" + ) if not self._indexed: raise exceptions.BadFilterError( - 'Cannot query for unindexed StructuredProperty %s' % self._name) + "Cannot query for unindexed StructuredProperty %s" % self._name + ) # Import late to avoid circular imports. from .query import ConjunctionNode, PostFilterNode from .query import RepeatedStructuredPropertyPredicate + if value is None: - from .query import FilterNode # Import late to avoid circular imports. + from .query import ( + FilterNode, + ) # Import late to avoid circular imports. + return FilterNode(self._name, op, value) value = self._do_validate(value) value = self._call_to_base_type(value) @@ -3425,20 +3435,22 @@ def _comparison(self, op, value): for prop in self._modelclass._properties.values(): vals = prop._get_base_value_unwrapped_as_list(value) if prop._repeated: - if vals: + if vals: # pragma: no branch raise exceptions.BadFilterError( - 'Cannot query for non-empty repeated property %s' % prop._name) - continue - assert isinstance(vals, list) and len(vals) == 1, repr(vals) + "Cannot query for non-empty repeated property %s" + % prop._name + ) + continue # pragma: NO COVER val = vals[0] - if val is not None: + if val is not None: # pragma: no branch altprop = getattr(self, prop._code_name) filt = altprop._comparison(op, val) filters.append(filt) match_keys.append(altprop._name) if not filters: raise exceptions.BadFilterError( - 'StructuredProperty filter without any values') + "StructuredProperty filter without any values" + ) if len(filters) == 1: return filters[0] if self._repeated: @@ -3452,10 +3464,12 @@ def _comparison(self, op, value): def _IN(self, value): if not isinstance(value, (list, tuple, set, frozenset)): raise exceptions.BadArgumentError( - 'Expected list, tuple or set, got %r' % (value,)) + "Expected list, tuple or set, got %r" % (value,) + ) from .query import DisjunctionNode, FalseNode + # Expand to a series of == filters. - filters = [self._comparison('=', val) for val in value] + filters = [self._comparison(query_module._EQ_OP, val) for val in value] if not filters: # DisjunctionNode doesn't like an empty list of filters. # Running the query will still fail, but this matches the @@ -3463,6 +3477,7 @@ def _IN(self, value): return FalseNode() else: return DisjunctionNode(*filters) + IN = _IN def _validate(self, value): @@ -3470,24 +3485,33 @@ def _validate(self, value): # A dict is assumed to be the result of a _to_dict() call. return self._modelclass(**value) if not isinstance(value, self._modelclass): - raise exceptions.BadValueError('Expected %s instance, got %s' % - (self._modelclass.__name__, value.__class__)) + raise exceptions.BadValueError( + "Expected %s instance, got %s" + % (self._modelclass.__name__, value.__class__) + ) def _has_value(self, entity, rest=None): - # rest: optional list of attribute names to check in addition. - # Basically, prop._has_value(self, ent, ['x', 'y']) is similar to - # (prop._has_value(ent) and - # prop.x._has_value(ent.x) and - # prop.x.y._has_value(ent.x.y)) - # assuming prop.x and prop.x.y exist. - # NOTE: This is not particularly efficient if len(rest) > 1, - # but that seems a rare case, so for now I don't care. + """Check if entity has a value for this property. + + Basically, prop._has_value(self, ent, ['x', 'y']) is similar to + (prop._has_value(ent) and prop.x._has_value(ent.x) and + prop.x.y._has_value(ent.x.y)), assuming prop.x and prop.x.y exist. + + Args: + entity (ndb.Model): An instance of a model. + rest (list[str]): optional list of attribute names to check in addition. + + Returns: + bool: True if the entity has a value for that property. + """ ok = super(StructuredProperty, self)._has_value(entity) if ok and rest: lst = self._get_base_value_unwrapped_as_list(entity) if len(lst) != 1: - raise RuntimeError('Failed to retrieve sub-entity of StructuredProperty' - ' %s' % self._name) + raise RuntimeError( + "Failed to retrieve sub-entity of StructuredProperty" + " %s" % self._name + ) subent = lst[0] if subent is None: return True @@ -3498,23 +3522,6 @@ def _has_value(self, entity, rest=None): ok = subprop._has_value(subent, rest[1:]) return ok - def _prepare_for_put(self, entity): - values = self._get_base_value_unwrapped_as_list(entity) - for value in values: - if value is not None: - value._prepare_for_put() - - def _check_property(self, rest=None, require_indexed=True): - """Override for Property._check_property(). - Raises: - InvalidPropertyError if no subproperty is specified or if something - is wrong with the subproperty. - """ - if not rest: - raise InvalidPropertyError( - 'Structured property %s requires a subproperty' % self._name) - self._modelclass._check_properties([rest], require_indexed=require_indexed) - def _get_base_value_at_index(self, entity, index): assert self._repeated value = self._retrieve_value(entity, self._default) @@ -3635,13 +3642,14 @@ class GenericProperty(Property): def __init__(self, name=None, compressed=False, **kwargs): if compressed: # Compressed implies unindexed. - kwargs.setdefault('indexed', False) + kwargs.setdefault("indexed", False) super(GenericProperty, self).__init__(name=name, **kwargs) self._compressed = compressed if compressed and self._indexed: - # TODO: Allow this, but only allow == and IN comparisons? - raise NotImplementedError('GenericProperty %s cannot be compressed and ' - 'indexed at the same time.' % self._name) + raise NotImplementedError( + "GenericProperty %s cannot be compressed and " + "indexed at the same time." % self._name + ) def _to_base_type(self, value): if self._compressed and isinstance(value, bytes): @@ -3655,8 +3663,9 @@ def _validate(self, value): if self._indexed: if isinstance(value, bytes) and len(value) > _MAX_STRING_LENGTH: raise exceptions.BadValueError( - 'Indexed value %s must be at most %d bytes' % - (self._name, _MAX_STRING_LENGTH)) + "Indexed value %s must be at most %d bytes" + % (self._name, _MAX_STRING_LENGTH) + ) def _db_get_value(self, v, unused_p): """Helper for :meth:`_deserialize`. @@ -3688,32 +3697,37 @@ class ComputedProperty(GenericProperty): Example: - >>> class DatastoreFile(Model): - ... name = StringProperty() - ... name_lower = ComputedProperty(lambda self: self.name.lower()) + >>> class DatastoreFile(ndb.Model): + ... name = ndb.model.StringProperty() + ... name_lower = ndb.model.ComputedProperty(lambda self: self.name.lower()) ... - ... data = BlobProperty() + ... data = ndb.model.BlobProperty() ... - ... @ComputedProperty + ... @ndb.model.ComputedProperty ... def size(self): ... return len(self.data) ... ... def _compute_hash(self): ... return hashlib.sha1(self.data).hexdigest() - ... hash = ComputedProperty(_compute_hash, name='sha1') + ... hash = ndb.model.ComputedProperty(_compute_hash, name='sha1') """ - def __init__(self, func, name=None, indexed=None, - repeated=None, verbose_name=None): + def __init__( + self, func, name=None, indexed=None, repeated=None, verbose_name=None + ): """Constructor. + Args: func: A function that takes one argument, the model instance, and returns a calculated value. """ - super(ComputedProperty, self).__init__(name=name, indexed=indexed, - repeated=repeated, - verbose_name=verbose_name) + super(ComputedProperty, self).__init__( + name=name, + indexed=indexed, + repeated=repeated, + verbose_name=verbose_name, + ) self._func = func def _set_value(self, entity, value): diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index b823dd32..0e267beb 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -2634,7 +2634,10 @@ class MineToo(model.Model): mine = Mine(foo="Foo") minetoo = MineToo() minetoo.bar = [mine, mine] - assert MineToo.bar._get_for_dict(minetoo) == [{"foo": "Foo"}, {"foo": "Foo"}] + assert MineToo.bar._get_for_dict(minetoo) == [ + {"foo": "Foo"}, + {"foo": "Foo"}, + ] @staticmethod def test__get_for_dict_no_value(): @@ -2644,7 +2647,6 @@ class Mine(model.Model): class MineToo(model.Model): bar = model.StructuredProperty(Mine) - mine = Mine(foo="Foo") minetoo = MineToo() minetoo.bar = None assert MineToo.bar._get_for_dict(minetoo) is None @@ -2676,7 +2678,9 @@ class Mine(model.Model): prop = model.StructuredProperty(Mine) prop._name = "bar" mine = Mine(foo="baz") - assert prop._comparison("=", mine) == query_module.FilterNode("bar.foo", "=", "baz") + assert prop._comparison("=", mine) == query_module.FilterNode( + "bar.foo", "=", "baz" + ) @staticmethod def test__comparison_other(): @@ -2705,7 +2709,9 @@ class Mine(model.Model): prop = model.StructuredProperty(Mine) prop._name = "bar" - assert prop._comparison("=", None) == query_module.FilterNode("bar", "=", None) + assert prop._comparison("=", None) == query_module.FilterNode( + "bar", "=", None + ) @staticmethod def test__comparison_repeated(): @@ -2716,7 +2722,9 @@ class Mine(model.Model): prop = model.StructuredProperty(Mine) prop._name = "baz" mine = Mine(bar="x") - assert prop._comparison("=", mine) == query_module.FilterNode("baz.bar", "=", "x") + assert prop._comparison("=", mine) == query_module.FilterNode( + "baz.bar", "=", "x" + ) @staticmethod def test__comparison_repeated_no_filters(): @@ -2740,6 +2748,17 @@ class Mine(model.Model): with pytest.raises(exceptions.BadFilterError): prop._comparison("=", mine) + @staticmethod + def test__comparison_repeated_empty(): + class Mine(model.Model): + foo = model.StringProperty(repeated=True) + + prop = model.StructuredProperty(Mine) + prop._name = "bar" + mine = Mine(foo=[]) + with pytest.raises(exceptions.BadFilterError): + prop._comparison("=", mine) + @staticmethod def test__comparison_multiple(): class Mine(model.Model): @@ -2751,7 +2770,7 @@ class Mine(model.Model): mine = Mine(foo="x", bar="y") assert prop._comparison("=", mine) == query_module.AND( query_module.FilterNode("baz.bar", "=", "y"), - query_module.FilterNode("baz.foo", "=", "x") + query_module.FilterNode("baz.foo", "=", "x"), ) @staticmethod @@ -2787,8 +2806,6 @@ class Mine(model.Model): prop = model.StructuredProperty(Mine) prop._name = "baz" - mine = Mine(foo="x") - minetoo = Mine(foo="y") assert prop.IN([]) == query_module.FalseNode() @staticmethod @@ -2825,6 +2842,31 @@ class MineToo(model.Model): minetoo = MineToo(bar=mine) assert MineToo.bar._has_value(minetoo, rest=["foo"]) is True + @staticmethod + def test__has_value_with_rest_subent_none(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + minetoo = MineToo(bar=None) + assert MineToo.bar._has_value(minetoo, rest=["foo"]) is True + + @staticmethod + def test__has_value_with_rest_repeated_none(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine, repeated=True) + + mine = Mine(foo="x") + mine2 = Mine(foo="y") + minetoo = MineToo(bar=[mine, mine2]) + with pytest.raises(RuntimeError): + MineToo.bar._has_value(minetoo, rest=["foo"]) + @staticmethod def test__has_value_with_rest_subprop_none(): class Mine(model.Model): @@ -2862,6 +2904,18 @@ class MineToo(model.Model): minetoo = MineToo(bar=mine) assert MineToo.bar._get_value_size(minetoo) == 1 + @staticmethod + def test__get_value_size_list(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine, repeated=True) + + mine = Mine(foo="Foo") + minetoo = MineToo(bar=[mine]) + assert MineToo.bar._get_value_size(minetoo) == 1 + @staticmethod def test__get_value_size_none(): class Mine(model.Model): @@ -2870,7 +2924,6 @@ class Mine(model.Model): class MineToo(model.Model): bar = model.StructuredProperty(Mine) - mine = Mine(foo="Foo") minetoo = MineToo(bar=None) assert MineToo.bar._get_value_size(minetoo) == 0 @@ -2991,7 +3044,9 @@ def test_constructor_compressed(): @staticmethod def test_constructor_compressed_and_indexed(): with pytest.raises(NotImplementedError): - model.GenericProperty(name="generic", compressed=True, indexed=True) + model.GenericProperty( + name="generic", compressed=True, indexed=True + ) @staticmethod def test__db_get_value(): @@ -3041,7 +3096,7 @@ def test__from_base_type_no_convert(): @staticmethod def test__validate(): - prop = model.GenericProperty(name="generic") + prop = model.GenericProperty(name="generic", indexed=False) assert prop._validate(b"abc") is None @staticmethod @@ -3070,20 +3125,20 @@ class TestComputedProperty: @staticmethod def test_constructor(): def lower_name(self): - return self.lower() # pragma: NO COVER + return self.lower() # pragma: NO COVER prop = model.ComputedProperty(lower_name) assert prop._func == lower_name @staticmethod def test__set_value(): - prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER + prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER with pytest.raises(model.ComputedPropertyError): prop._set_value(None, None) @staticmethod def test__delete_value(): - prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER + prop = model.ComputedProperty(lambda self: self) # pragma: NO COVER with pytest.raises(model.ComputedPropertyError): prop._delete_value(None) @@ -3097,9 +3152,13 @@ def test__get_value(): @staticmethod def test__get_value_with_projection(): - prop = model.ComputedProperty(lambda self: 42, name="computed") # pragma: NO COVER + prop = model.ComputedProperty( + lambda self: 42, name="computed" + ) # pragma: NO COVER entity = unittest.mock.Mock( - _projection=['computed'], _values={"computed": 84}, spec=("_projection", "_values") + _projection=["computed"], + _values={"computed": 84}, + spec=("_projection", "_values"), ) assert prop._get_value(entity) == 84 From f24e02e1f3e87d97de5c09754c0b7aaf82a872a1 Mon Sep 17 00:00:00 2001 From: Carlos de la Guardia Date: Fri, 24 May 2019 04:07:22 -0500 Subject: [PATCH 4/4] add back unintentionally removed code and add tests for it --- src/google/cloud/ndb/model.py | 24 ++++++++++++++++++- tests/unit/test_model.py | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/google/cloud/ndb/model.py b/src/google/cloud/ndb/model.py index 414c9d69..73445b39 100644 --- a/src/google/cloud/ndb/model.py +++ b/src/google/cloud/ndb/model.py @@ -3377,7 +3377,14 @@ def __init__(self, modelclass, name=None, **kwargs): self._modelclass = modelclass def _get_value(self, entity): - """Override _get_value() to *not* raise UnprojectedPropertyError.""" + """Override _get_value() to *not* raise UnprojectedPropertyError. + + This is necessary because the projection must include both the sub-entity and + the property name that is projected (e.g. 'foo.bar' instead of only 'foo'). In + that case the original code would fail, because it only looks for the property + name ('foo'). Here we check for a value, and only call the original code if the + value is None. + """ value = self._get_user_value(entity) if value is None and entity._projection: # Invoke super _get_value() to raise the proper exception. @@ -3522,6 +3529,21 @@ def _has_value(self, entity, rest=None): ok = subprop._has_value(subent, rest[1:]) return ok + def _check_property(self, rest=None, require_indexed=True): + """Override for Property._check_property(). + + Raises: + InvalidPropertyError if no subproperty is specified or if something + is wrong with the subproperty. + """ + if not rest: + raise InvalidPropertyError( + "Structured property %s requires a subproperty" % self._name + ) + self._modelclass._check_properties( + [rest], require_indexed=require_indexed + ) + def _get_base_value_at_index(self, entity, index): assert self._repeated value = self._retrieve_value(entity, self._default) diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 0e267beb..fd86ef0f 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -2879,6 +2879,51 @@ class MineToo(model.Model): minetoo = MineToo(bar=mine) assert MineToo.bar._has_value(minetoo, rest=[None]) is False + @staticmethod + def test__check_property(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + assert MineToo.bar._check_property("foo") is None + + @staticmethod + def test__check_property_with_sub(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + class MineThree(model.Model): + baz = model.StructuredProperty(MineToo) + + assert MineThree.baz._check_property("bar.foo") is None + + @staticmethod + def test__check_property_invalid(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + with pytest.raises(model.InvalidPropertyError): + MineToo.bar._check_property("baz") + + @staticmethod + def test__check_property_no_rest(): + class Mine(model.Model): + foo = model.StringProperty() + + class MineToo(model.Model): + bar = model.StructuredProperty(Mine) + + with pytest.raises(model.InvalidPropertyError): + MineToo.bar._check_property() + @staticmethod def test__get_base_value_at_index(): class Mine(model.Model):