From 990b3fdc32fcb06fa6e2fa78668bb5bbd2329796 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:20:23 -0700 Subject: [PATCH 1/7] Implementing `Property._get_value`. --- ndb/src/google/cloud/ndb/model.py | 25 +++++++++++++++++++++ ndb/tests/unit/test_model.py | 37 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index 9238851f7e51..82dc39d691e6 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1190,6 +1190,31 @@ def _apply_to_values(self, entity, function): return value + def _get_value(self, entity): + """Get the value for this property from an entity. + + For a repeated property this initializes the value to an empty + list if it is not set. + + Args: + entity (Model): An entity to get a value from. + + Returns: + Any: The user value stored for the current property. + + Raises: + UnprojectedPropertyError: If the ``entity`` is the result of a + projection query and the current property is not one of the + projected properties. + """ + if entity._projection: + if self._name not in entity._projection: + raise UnprojectedPropertyError( + "Property {} is not in the projection".format(self._name) + ) + + return self._get_user_value(entity) + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index 9f28ff6268e4..cdfd9e723db3 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1151,6 +1151,43 @@ def test__apply_to_values_repeated_when_none(): # Check mocks. function.assert_not_called() + @staticmethod + def test__get_value(): + prop = model.Property(name="prop") + value = b"\x00\x01" + values = {prop._name: value} + entity = unittest.mock.Mock( + _projection=(), _values=values, spec=("_projection", "_values") + ) + assert value is prop._get_value(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + + @staticmethod + def test__get_value_projected_present(): + prop = model.Property(name="prop") + value = 92.5 + values = {prop._name: value} + entity = unittest.mock.Mock( + _projection=(prop._name,), + _values=values, + spec=("_projection", "_values"), + ) + assert value is prop._get_value(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + + @staticmethod + def test__get_value_projected_absent(): + prop = model.Property(name="prop") + entity = unittest.mock.Mock( + _projection=("nope",), spec=("_projection",) + ) + with pytest.raises(model.UnprojectedPropertyError): + prop._get_value(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + class TestModelKey: @staticmethod From 99088d602070109c520e60f23767f0ef74eed972 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:23:22 -0700 Subject: [PATCH 2/7] Implementing `Property._delete_value`. --- ndb/src/google/cloud/ndb/model.py | 15 +++++++++++++++ ndb/tests/unit/test_model.py | 17 +++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index 82dc39d691e6..92247be25924 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1215,6 +1215,21 @@ def _get_value(self, entity): return self._get_user_value(entity) + def _delete_value(self, entity): + """Delete the value for this property from an entity. + + .. note:: + + If no value exists this is a no-op; deleted values will not be + serialized but requesting their value will return :data:`None` (or + an empty list in the case of a repeated property). + + Args: + entity (Model): An entity to get a value from. + """ + if self._name in entity._values: + del entity._values[self._name] + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index cdfd9e723db3..f389435b9e04 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1188,6 +1188,23 @@ def test__get_value_projected_absent(): # Cache is untouched. assert model.Property._FIND_METHODS_CACHE == {} + @staticmethod + def test__delete_value(): + prop = model.Property(name="prop") + value = b"\x00\x01" + values = {prop._name: value} + entity = unittest.mock.Mock(_values=values, spec=("_values",)) + prop._delete_value(entity) + assert values == {} + + @staticmethod + def test__delete_value_no_op(): + prop = model.Property(name="prop") + values = {} + entity = unittest.mock.Mock(_values=values, spec=("_values",)) + prop._delete_value(entity) + assert values == {} + class TestModelKey: @staticmethod From 6715708eca504eda3dfd5b09620ccaaf14bce863 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:30:58 -0700 Subject: [PATCH 3/7] Implementing `Property._is_initialized`. --- ndb/src/google/cloud/ndb/model.py | 14 ++++++++++++++ ndb/tests/unit/test_model.py | 32 ++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index 92247be25924..5ec40f4c3ac1 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1230,6 +1230,20 @@ def _delete_value(self, entity): if self._name in entity._values: del entity._values[self._name] + def _is_initialized(self, entity): + """Ask if the entity has a value for this property. + + This returns :data:`False` if a value is stored but the stored value + is :data:`None`. + + Args: + entity (Model): An entity to get a value from. + """ + return not self._required or ( + (self._has_value(entity) or self._default is not None) + and self._get_value(entity) is not None + ) + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index f389435b9e04..be9f877c1ba8 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1157,7 +1157,7 @@ def test__get_value(): value = b"\x00\x01" values = {prop._name: value} entity = unittest.mock.Mock( - _projection=(), _values=values, spec=("_projection", "_values") + _projection=None, _values=values, spec=("_projection", "_values") ) assert value is prop._get_value(entity) # Cache is untouched. @@ -1205,6 +1205,36 @@ def test__delete_value_no_op(): prop._delete_value(entity) assert values == {} + @staticmethod + def test__is_initialized_not_required(): + prop = model.Property(name="prop", required=False) + entity = unittest.mock.sentinel.entity + assert prop._is_initialized(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + + @staticmethod + def test__is_initialized_default_fallback(): + prop = model.Property(name="prop", required=True, default=11111) + values = {} + entity = unittest.mock.Mock( + _projection=None, _values=values, spec=("_projection", "_values") + ) + assert prop._is_initialized(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + + @staticmethod + def test__is_initialized_set_to_none(): + prop = model.Property(name="prop", required=True) + values = {prop._name: None} + entity = unittest.mock.Mock( + _projection=None, _values=values, spec=("_projection", "_values") + ) + assert not prop._is_initialized(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + class TestModelKey: @staticmethod From 67f3b34da0245c9098cad55d1e4644624b1524ec Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:36:06 -0700 Subject: [PATCH 4/7] Adding descriptors for `Property`. --- ndb/src/google/cloud/ndb/model.py | 16 ++++++++++++++++ ndb/tests/unit/test_model.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index 5ec40f4c3ac1..bb6c3a923730 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1244,6 +1244,22 @@ def _is_initialized(self, entity): and self._get_value(entity) is not None ) + def __get__(self, entity, unused_cls=None): + """Descriptor protocol: get the value from the entity.""" + if entity is None: + # Handle the case where ``__get__`` is called on the class + # rather than an instance. + return self + return self._get_value(entity) + + def __set__(self, entity, value): + """Descriptor protocol: set the value on the entity.""" + self._set_value(entity, value) + + def __delete__(self, entity): + """Descriptor protocol: delete the value from the entity.""" + self._delete_value(entity) + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index be9f877c1ba8..f32cc84edab0 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1235,6 +1235,35 @@ def test__is_initialized_set_to_none(): # Cache is untouched. assert model.Property._FIND_METHODS_CACHE == {} + @staticmethod + def test_instance_descriptors(): + class Model: + prop = model.Property(name="prop", required=True) + + def __init__(self): + self._projection = None + self._values = {} + + m = Model() + value = 1234.5 + # __set__ + m.prop = value + assert m._values == {b"prop": value} + # __get__ + assert m.prop == value + # __delete__ + del m.prop + assert m._values == {} + + @staticmethod + def test_class_descriptors(): + prop = model.Property(name="prop", required=True) + + class Model: + prop2 = prop + + assert Model.prop2 is prop + class TestModelKey: @staticmethod From 11c9648b4523ce735acd505bbd34ee751e34c59d Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:47:28 -0700 Subject: [PATCH 5/7] Implementing `Property._prepare_for_put`. --- ndb/src/google/cloud/ndb/model.py | 3 +++ ndb/tests/unit/test_model.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index bb6c3a923730..6de7eb910afc 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1260,6 +1260,9 @@ def __delete__(self, entity): """Descriptor protocol: delete the value from the entity.""" self._delete_value(entity) + def _prepare_for_put(self, entity): + pass + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index f32cc84edab0..558dc15c9fa2 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1264,6 +1264,11 @@ class Model: assert Model.prop2 is prop + @staticmethod + def test__prepare_for_put(): + prop = model.Property(name="prop") + assert prop._prepare_for_put(None) is None + class TestModelKey: @staticmethod From 2000cf12e270d51288c8d10721d46ca29cd7f4f6 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 14:55:21 -0700 Subject: [PATCH 6/7] Implementing `Property._check_property`. Also filling in some missing / incomplete docstrings on recently implemented `Property` methods. --- ndb/src/google/cloud/ndb/model.py | 57 +++++++++++++++++++++++++++++-- ndb/tests/unit/test_model.py | 17 +++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index 6de7eb910afc..d41be3b13480 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1245,7 +1245,12 @@ def _is_initialized(self, entity): ) def __get__(self, entity, unused_cls=None): - """Descriptor protocol: get the value from the entity.""" + """Descriptor protocol: get the value from the entity. + + Args: + entity (Model): An entity to get a value from. + unused_cls (type): The class that owns this instance. + """ if entity is None: # Handle the case where ``__get__`` is called on the class # rather than an instance. @@ -1253,16 +1258,62 @@ def __get__(self, entity, unused_cls=None): return self._get_value(entity) def __set__(self, entity, value): - """Descriptor protocol: set the value on the entity.""" + """Descriptor protocol: set the value on the entity. + + Args: + entity (Model): An entity to set a value on. + value (Any): The value to set. + """ self._set_value(entity, value) def __delete__(self, entity): - """Descriptor protocol: delete the value from the entity.""" + """Descriptor protocol: delete the value from the entity. + + Args: + entity (Model): An entity to delete a value from. + """ self._delete_value(entity) def _prepare_for_put(self, entity): + """Allow this property to define a pre-put hook. + + This base class implementation does nothing, but subclasses may + provide hooks. + + Args: + entity (Model): An entity with values. + """ pass + def _check_property(self, rest=None, require_indexed=True): + """Check this property for specific requirements. + + Called by ``Model._check_properties()``. + + Args: + rest: Optional subproperty to check, of the form + ``name1.name2...nameN``. + required_indexed (bool): Indicates if the current property must + be indexed. + + Raises: + InvalidPropertyError: If ``require_indexed`` is :data:`True` + but the current property is not indexed. + InvalidPropertyError: If a subproperty is specified via ``rest`` + (:class:`StructuredProperty` overrides this method to handle + subproperties). + """ + if require_indexed and not self._indexed: + raise InvalidPropertyError( + "Property is unindexed {}".format(self._name) + ) + + if rest: + raise InvalidPropertyError( + "Referencing subproperty {}.{} but {} is not a structured " + "property".format(self._name, rest, self._name) + ) + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index 558dc15c9fa2..d3a3fa111f91 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1269,6 +1269,23 @@ def test__prepare_for_put(): prop = model.Property(name="prop") assert prop._prepare_for_put(None) is None + @staticmethod + def test__check_property(): + prop = model.Property(name="prop") + assert prop._check_property() is None + + @staticmethod + def test__check_property_not_indexed(): + prop = model.Property(name="prop", indexed=False) + with pytest.raises(model.InvalidPropertyError): + prop._check_property(require_indexed=True) + + @staticmethod + def test__check_property_with_subproperty(): + prop = model.Property(name="prop", indexed=True) + with pytest.raises(model.InvalidPropertyError): + prop._check_property(rest="a.b.c") + class TestModelKey: @staticmethod From 90a6cb0f2be4f18d3796f41ac8d131ba58002b77 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 29 Oct 2018 15:18:29 -0700 Subject: [PATCH 7/7] Implementing `Property._get_for_dict`. Also cleaning up the `Property._FIND_METHODS_CACHE` in `test_instance_descriptors()`. --- ndb/src/google/cloud/ndb/model.py | 27 +++++++++++++++++++++++++++ ndb/tests/unit/test_model.py | 14 +++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ndb/src/google/cloud/ndb/model.py b/ndb/src/google/cloud/ndb/model.py index d41be3b13480..1d2dbb1f86fe 100644 --- a/ndb/src/google/cloud/ndb/model.py +++ b/ndb/src/google/cloud/ndb/model.py @@ -1314,6 +1314,33 @@ def _check_property(self, rest=None, require_indexed=True): "property".format(self._name, rest, self._name) ) + def _get_for_dict(self, entity): + """Retrieve the value like ``_get_value()``. + + This is intended to be processed for ``_to_dict()``. + + Property subclasses can override this if they want the dictionary + returned by ``entity._to_dict()`` to contain a different value. The + main use case is allowing :class:`StructuredProperty` and + :class:`LocalStructuredProperty` to allow the default ``_get_value()`` + behavior. + + * If you override ``_get_for_dict()`` to return a different type, you + must override ``_validate()`` to accept values of that type and + convert them back to the original type. + + * If you override ``_get_for_dict()``, you must handle repeated values + and :data:`None` correctly. However, ``_validate()`` does not need to + handle these. + + Args: + entity (Model): An entity to get a value from. + + Returns: + Any: The user value stored for the current property. + """ + return self._get_value(entity) + class ModelKey(Property): __slots__ = () diff --git a/ndb/tests/unit/test_model.py b/ndb/tests/unit/test_model.py index d3a3fa111f91..fe625d32c9f1 100644 --- a/ndb/tests/unit/test_model.py +++ b/ndb/tests/unit/test_model.py @@ -1236,7 +1236,7 @@ def test__is_initialized_set_to_none(): assert model.Property._FIND_METHODS_CACHE == {} @staticmethod - def test_instance_descriptors(): + def test_instance_descriptors(property_clean_cache): class Model: prop = model.Property(name="prop", required=True) @@ -1286,6 +1286,18 @@ def test__check_property_with_subproperty(): with pytest.raises(model.InvalidPropertyError): prop._check_property(rest="a.b.c") + @staticmethod + def test__get_for_dict(): + prop = model.Property(name="prop") + value = b"\x00\x01" + values = {prop._name: value} + entity = unittest.mock.Mock( + _projection=None, _values=values, spec=("_projection", "_values") + ) + assert value is prop._get_for_dict(entity) + # Cache is untouched. + assert model.Property._FIND_METHODS_CACHE == {} + class TestModelKey: @staticmethod