From ddaca14103cbf99ab56eb0413560634d281bd3e1 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 22 Dec 2014 16:39:31 -0800 Subject: [PATCH] Making all attributes on Key read-only and sorts out use of setters. Addresses second part of #451. In some cases, since no valid replacements for setters are implemented here, the functionality is replaced with hacks using protected methods. Also added a getter for Key._dataset_id and made the Key.path property return a copy since a `list` of `dict`s is very much mutable. --- gcloud/datastore/dataset.py | 16 ++- gcloud/datastore/entity.py | 13 ++- gcloud/datastore/key.py | 155 ++++++++++++--------------- gcloud/datastore/test___init__.py | 2 +- gcloud/datastore/test_dataset.py | 23 ++-- gcloud/datastore/test_entity.py | 54 +++++----- gcloud/datastore/test_helpers.py | 26 ++--- gcloud/datastore/test_key.py | 125 +++++---------------- gcloud/datastore/test_query.py | 2 +- gcloud/datastore/test_transaction.py | 5 +- gcloud/datastore/transaction.py | 9 +- regression/clear_datastore.py | 2 +- regression/datastore.py | 36 +++---- 13 files changed, 194 insertions(+), 274 deletions(-) diff --git a/gcloud/datastore/dataset.py b/gcloud/datastore/dataset.py index fb0bec2d4325b..ed5e85ef225ab 100644 --- a/gcloud/datastore/dataset.py +++ b/gcloud/datastore/dataset.py @@ -186,7 +186,7 @@ def allocate_ids(self, incomplete_key, num_ids): :returns: The (complete) keys allocated with `incomplete_key` as root. :raises: `ValueError` if `incomplete_key` is not a partial key. """ - if not incomplete_key.is_partial(): + if not incomplete_key.is_partial: raise ValueError(('Key is not partial.', incomplete_key)) incomplete_key_pb = incomplete_key.to_protobuf() @@ -196,5 +196,17 @@ def allocate_ids(self, incomplete_key, num_ids): self.id(), incomplete_key_pbs) allocated_ids = [allocated_key_pb.path_element[-1].id for allocated_key_pb in allocated_key_pbs] - return [incomplete_key.id(allocated_id) + + # This method is temporary and will move over to Key in + # part 5 of #451. + def create_new_key(new_id): + """Temporary method to complete `incomplete_key`. + + Uses `incomplete_key` from outside scope. + """ + clone = incomplete_key._clone() + clone._path[-1]['id'] = new_id + return clone + + return [create_new_key(allocated_id) for allocated_id in allocated_ids] diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py index 4e5bfa1f87dec..f50f8769432dc 100644 --- a/gcloud/datastore/entity.py +++ b/gcloud/datastore/entity.py @@ -99,7 +99,7 @@ def __init__(self, dataset=None, kind=None, exclude_from_indexes=()): self._dataset = dataset or _implicit_environ.DATASET self._data = {} if kind: - self._key = Key().kind(kind) + self._key = Key(path=[{'kind': kind}]) else: self._key = None self._exclude_from_indexes = set(exclude_from_indexes) @@ -195,7 +195,7 @@ def kind(self): """ if self._key: - return self._key.kind() + return self._key.kind def exclude_from_indexes(self): """Names of fields which are *not* to be indexed for this entity. @@ -292,7 +292,7 @@ def save(self): # If we are in a transaction and the current entity needs an # automatically assigned ID, tell the transaction where to put that. transaction = connection.transaction() - if transaction and key.is_partial(): + if transaction and key.is_partial: transaction.add_auto_id_entity(self) if isinstance(key_pb, datastore_pb.Key): @@ -308,7 +308,10 @@ def save(self): for descriptor, value in element._fields.items(): key_part[descriptor.name] = value path.append(key_part) - self._key = key.path(path) + # This is temporary. Will be addressed throughout #451. + clone = key._clone() + clone._path = path + self._key = clone return self @@ -329,6 +332,6 @@ def delete(self): def __repr__(self): if self._key: - return '' % (self._key.path(), self._data) + return '' % (self._key.path, self._data) else: return '' % (self._data,) diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py index cfecdbb076afe..23de613564024 100644 --- a/gcloud/datastore/key.py +++ b/gcloud/datastore/key.py @@ -70,13 +70,13 @@ def to_protobuf(self): """ key = datastore_pb.Key() - if self._dataset_id is not None: - key.partition_id.dataset_id = self._dataset_id + if self.dataset_id is not None: + key.partition_id.dataset_id = self.dataset_id if self._namespace: key.partition_id.namespace = self._namespace - for item in self.path(): + for item in self.path: element = key.path_element.add() if 'kind' in item: element.kind = item['kind'] @@ -87,121 +87,100 @@ def to_protobuf(self): return key + @property def is_partial(self): - """Boolean test: is the key fully mapped onto a backend entity? + """Boolean indicating if the key has an ID (or name). :rtype: :class:`bool` :returns: True if the last element of the key's path does not have an 'id' or a 'name'. """ - return self.id_or_name() is None + return self.id_or_name is None - def namespace(self, namespace=None): - """Namespace setter / getter. + @property + def namespace(self): + """Namespace getter. - :type namespace: :class:`str` - :param namespace: A namespace identifier for the key. - - :rtype: :class:`Key` (for setter); or :class:`str` (for getter) - :returns: a new key, cloned from self., with the given namespace - (setter); or self's namespace (getter). + :rtype: :class:`str` + :returns: The namespace of the current key. """ - if namespace: - clone = self._clone() - clone._namespace = namespace - return clone - else: - return self._namespace + return self._namespace - def path(self, path=None): - """Path setter / getter. + @property + def path(self): + """Path getter. - :type path: sequence of dicts - :param path: Each dict must have keys 'kind' (a string) and optionally - 'name' (a string) or 'id' (an integer). + Returns a copy so that the key remains immutable. - :rtype: :class:`Key` (for setter); or :class:`str` (for getter) - :returns: a new key, cloned from self., with the given path (setter); - or self's path (getter). + :rtype: :class:`str` + :returns: The (key) path of the current key. """ - if path: - clone = self._clone() - clone._path = path - return clone - else: - return self._path - - def kind(self, kind=None): - """Kind setter / getter. Based on the last element of path. - - :type kind: :class:`str` - :param kind: The new kind for the key. - - :rtype: :class:`Key` (for setter); or :class:`str` (for getter) - :returns: a new key, cloned from self., with the given kind (setter); - or self's kind (getter). + return copy.deepcopy(self._path) + + @property + def kind(self): + """Kind getter. Based on the last element of path. + + :rtype: :class:`str` + :returns: The kind of the current key. """ - if kind: - clone = self._clone() - clone._path[-1]['kind'] = kind - return clone - elif self.path(): - return self._path[-1]['kind'] - - def id(self, id_to_set=None): - """ID setter / getter. Based on the last element of path. - - :type id_to_set: :class:`int` - :param id_to_set: The new ID for the key. - - :rtype: :class:`Key` (for setter); or :class:`int` (for getter) - :returns: a new key, cloned from self., with the given id (setter); - or self's id (getter). + if self.path: + return self.path[-1].get('kind') + + @property + def id(self): + """ID getter. Based on the last element of path. + + :rtype: :class:`int` + :returns: The (integer) ID of the key. """ - if id_to_set: - clone = self._clone() - clone._path[-1]['id'] = id_to_set - return clone - elif self.path(): - return self._path[-1].get('id') - - def name(self, name=None): - """Name setter / getter. Based on the last element of path. - - :type kind: :class:`str` - :param kind: The new name for the key. - - :rtype: :class:`Key` (for setter); or :class:`str` (for getter) - :returns: a new key, cloned from self., with the given name (setter); - or self's name (getter). + if self.path: + return self.path[-1].get('id') + + @property + def name(self): + """Name getter. Based on the last element of path. + + :rtype: :class:`str` + :returns: The (string) name of the key. """ - if name: - clone = self._clone() - clone._path[-1]['name'] = name - return clone - elif self.path(): - return self._path[-1].get('name') + if self.path: + return self.path[-1].get('name') + @property def id_or_name(self): - """Getter. Based on the last element of path. + """Getter. Based on the last element of path. - :rtype: :class:`int` (if 'id' is set); or :class:`str` (the 'name') - :returns: True if the last element of the key's path has either an 'id' + :rtype: :class:`int` (if 'id') or :class:`str` (if 'name') + :returns: The last element of the key's path if it is either an 'id' or a 'name'. """ - return self.id() or self.name() + return self.id or self.name + + @property + def dataset_id(self): + """Dataset ID getter. + + :rtype: :class:`str` + :returns: The key's dataset. + """ + return self._dataset_id + @property def parent(self): """Getter: return a new key for the next highest element in path. :rtype: :class:`gcloud.datastore.key.Key` :returns: a new `Key` instance, whose path consists of all but the last element of self's path. If self has only one path element, - return None. + returns None. """ if len(self._path) <= 1: return None - return self.path(self.path()[:-1]) + # This is temporary. Will be addressed throughout #451. + clone = self._clone() + clone._path = self.path[:-1] + return clone def __repr__(self): - return '' % self.path() + return '' % self.path diff --git a/gcloud/datastore/test___init__.py b/gcloud/datastore/test___init__.py index 54369e06e8459..58d159f4cc34d 100644 --- a/gcloud/datastore/test___init__.py +++ b/gcloud/datastore/test___init__.py @@ -160,7 +160,7 @@ def test_allocate_ids(self): result = gcloud.datastore.allocate_ids(INCOMPLETE_KEY, NUM_IDS) # Check the IDs returned. - self.assertEqual([key.id() for key in result], range(1, NUM_IDS + 1)) + self.assertEqual([key.id for key in result], range(1, NUM_IDS + 1)) def test_set_DATASET(self): import os diff --git a/gcloud/datastore/test_dataset.py b/gcloud/datastore/test_dataset.py index d9d32b443bdd3..48a9edcf589cd 100644 --- a/gcloud/datastore/test_dataset.py +++ b/gcloud/datastore/test_dataset.py @@ -104,8 +104,8 @@ def test_get_entity_hit(self): key = Key(path=PATH) result = dataset.get_entity(key) key = result.key() - self.assertEqual(key._dataset_id, DATASET_ID) - self.assertEqual(key.path(), PATH) + self.assertEqual(key.dataset_id, DATASET_ID) + self.assertEqual(key.path, PATH) self.assertEqual(result.to_dict().keys(), ['foo']) self.assertEqual(result['foo'], 'Foo') @@ -175,19 +175,15 @@ def test_get_entities_hit(self): key = Key(path=PATH) result, = dataset.get_entities([key]) key = result.key() - self.assertEqual(key._dataset_id, DATASET_ID) - self.assertEqual(key.path(), PATH) + self.assertEqual(key.dataset_id, DATASET_ID) + self.assertEqual(key.path, PATH) self.assertEqual(result.to_dict().keys(), ['foo']) self.assertEqual(result['foo'], 'Foo') def test_allocate_ids(self): - from gcloud.datastore.test_entity import _Key - - INCOMPLETE_KEY = _Key() - PROTO_ID = object() - INCOMPLETE_KEY._key = _KeyProto(PROTO_ID) - INCOMPLETE_KEY._partial = True + from gcloud.datastore.key import Key + INCOMPLETE_KEY = Key(path=[{'kind': 'foo'}]) CONNECTION = _Connection() NUM_IDS = 2 DATASET_ID = 'foo' @@ -195,17 +191,12 @@ def test_allocate_ids(self): result = DATASET.allocate_ids(INCOMPLETE_KEY, NUM_IDS) # Check the IDs returned match. - self.assertEqual([key._id for key in result], range(NUM_IDS)) + self.assertEqual([key.id for key in result], range(NUM_IDS)) # Check connection is called correctly. self.assertEqual(CONNECTION._called_dataset_id, DATASET_ID) self.assertEqual(len(CONNECTION._called_key_pbs), NUM_IDS) - # Check the IDs passed to Connection.allocate_ids. - key_paths = [key_pb.path_element[-1].id - for key_pb in CONNECTION._called_key_pbs] - self.assertEqual(key_paths, [PROTO_ID] * NUM_IDS) - def test_allocate_ids_with_complete(self): from gcloud.datastore.test_entity import _Key diff --git a/gcloud/datastore/test_entity.py b/gcloud/datastore/test_entity.py index be7fe6f8fa817..aa3ebfac69369 100644 --- a/gcloud/datastore/test_entity.py +++ b/gcloud/datastore/test_entity.py @@ -61,8 +61,8 @@ def test_key_getter(self): entity = self._makeOne() key = entity.key() self.assertIsInstance(key, Key) - self.assertEqual(key._dataset_id, None) - self.assertEqual(key.kind(), _KIND) + self.assertEqual(key.dataset_id, None) + self.assertEqual(key.kind, _KIND) def test_key_setter(self): entity = self._makeOne() @@ -127,13 +127,13 @@ def test_from_key_wo_dataset(self): from gcloud.datastore.key import Key klass = self._getTargetClass() - key = Key().kind(_KIND).id(_ID) + key = Key(path=[{'kind': _KIND, 'id': _ID}]) entity = klass.from_key(key) self.assertTrue(entity.dataset() is None) self.assertEqual(entity.kind(), _KIND) key = entity.key() - self.assertEqual(key.kind(), _KIND) - self.assertEqual(key.id(), _ID) + self.assertEqual(key.kind, _KIND) + self.assertEqual(key.id, _ID) def test_from_key_w_dataset(self): from gcloud.datastore.dataset import Dataset @@ -141,13 +141,13 @@ def test_from_key_w_dataset(self): klass = self._getTargetClass() dataset = Dataset(_DATASET_ID) - key = Key().kind(_KIND).id(_ID) + key = Key(path=[{'kind': _KIND, 'id': _ID}]) entity = klass.from_key(key, dataset) self.assertTrue(entity.dataset() is dataset) self.assertEqual(entity.kind(), _KIND) key = entity.key() - self.assertEqual(key.kind(), _KIND) - self.assertEqual(key.id(), _ID) + self.assertEqual(key.kind, _KIND) + self.assertEqual(key.id, _ID) def test__must_key_no_key(self): from gcloud.datastore.entity import NoKey @@ -246,21 +246,28 @@ def test_save_w_transaction_w_partial_key(self): def test_save_w_returned_key_exclude_from_indexes(self): from gcloud.datastore import datastore_v1_pb2 as datastore_pb + from gcloud.datastore.key import Key + key_pb = datastore_pb.Key() key_pb.partition_id.dataset_id = _DATASET_ID key_pb.path_element.add(kind=_KIND, id=_ID) connection = _Connection() connection._save_result = key_pb dataset = _Dataset(connection) - key = _Key() + key = Key() + # key_pb_before = key.to_protobuf() entity = self._makeOne(dataset, exclude_from_indexes=['foo']) entity.key(key) entity['foo'] = 'Foo' self.assertTrue(entity.save() is entity) self.assertEqual(entity['foo'], 'Foo') - self.assertEqual(connection._saved, - (_DATASET_ID, 'KEY', {'foo': 'Foo'}, ('foo',))) - self.assertEqual(key._path, [{'kind': _KIND, 'id': _ID}]) + self.assertEqual(connection._saved[0], _DATASET_ID) + self.assertEqual(connection._saved[1], key.to_protobuf()) + self.assertEqual(connection._saved[2], {'foo': 'Foo'}) + self.assertEqual(connection._saved[3], ('foo',)) + self.assertEqual(len(connection._saved), 4) + + self.assertEqual(entity.key()._path, [{'kind': _KIND, 'id': _ID}]) def test_delete_no_key(self): from gcloud.datastore.entity import NoKey @@ -287,7 +294,7 @@ def test___repr___w_key_non_empty(self): connection = _Connection() dataset = _Dataset(connection) key = _Key() - key.path('/bar/baz') + key._path = '/bar/baz' entity = self._makeOne(dataset) entity.key(key) entity['foo'] = 'Foo' @@ -301,22 +308,16 @@ class _Key(object): _path = None _id = None - def id(self, id_to_set): - self._called_id = id_to_set - clone = _Key() - clone._id = id_to_set - return clone - def to_protobuf(self): return self._key + @property def is_partial(self): return self._partial - def path(self, path=_MARKER): - if path is self._MARKER: - return self._path - self._path = path + @property + def path(self): + return self._path class _Dataset(dict): @@ -345,7 +346,12 @@ def get_entities(self, keys): return [self.get(key) for key in keys] def allocate_ids(self, incomplete_key, num_ids): - return [incomplete_key.id(i + 1) for i in range(num_ids)] + def clone_with_new_id(key, new_id): + clone = key._clone() + clone._path[-1]['id'] = new_id + return clone + return [clone_with_new_id(incomplete_key, i + 1) + for i in range(num_ids)] class _Connection(object): diff --git a/gcloud/datastore/test_helpers.py b/gcloud/datastore/test_helpers.py index d3727c3f0bc0b..a7230a7194560 100644 --- a/gcloud/datastore/test_helpers.py +++ b/gcloud/datastore/test_helpers.py @@ -47,10 +47,10 @@ def test_wo_dataset(self): self.assertEqual(entity.kind(), _KIND) self.assertEqual(entity['foo'], 'Foo') key = entity.key() - self.assertEqual(key._dataset_id, _DATASET_ID) - self.assertEqual(key.namespace(), None) - self.assertEqual(key.kind(), _KIND) - self.assertEqual(key.id(), _ID) + self.assertEqual(key.dataset_id, _DATASET_ID) + self.assertEqual(key.namespace, None) + self.assertEqual(key.kind, _KIND) + self.assertEqual(key.id, _ID) def test_w_dataset(self): from gcloud.datastore import datastore_v1_pb2 as datastore_pb @@ -71,10 +71,10 @@ def test_w_dataset(self): self.assertEqual(entity.kind(), _KIND) self.assertEqual(entity['foo'], 'Foo') key = entity.key() - self.assertEqual(key._dataset_id, _DATASET_ID) - self.assertEqual(key.namespace(), None) - self.assertEqual(key.kind(), _KIND) - self.assertEqual(key.id(), _ID) + self.assertEqual(key.dataset_id, _DATASET_ID) + self.assertEqual(key.namespace, None) + self.assertEqual(key.kind, _KIND) + self.assertEqual(key.id, _ID) class Test_key_from_protobuf(unittest2.TestCase): @@ -104,15 +104,15 @@ def test_w_dataset_id_in_pb(self): _DATASET = 'DATASET' pb = self._makePB(_DATASET) key = self._callFUT(pb) - self.assertEqual(key._dataset_id, _DATASET) - self.assertEqual(key.namespace(), None) + self.assertEqual(key.dataset_id, _DATASET) + self.assertEqual(key.namespace, None) def test_w_namespace_in_pb(self): _NAMESPACE = 'NAMESPACE' pb = self._makePB(namespace=_NAMESPACE) key = self._callFUT(pb) - self.assertEqual(key._dataset_id, None) - self.assertEqual(key.namespace(), _NAMESPACE) + self.assertEqual(key.dataset_id, None) + self.assertEqual(key.namespace, _NAMESPACE) def test_w_path_in_pb(self): _DATASET = 'DATASET' @@ -132,7 +132,7 @@ def test_w_path_in_pb(self): ] pb = self._makePB(path=_PATH) key = self._callFUT(pb) - self.assertEqual(key.path(), _PATH) + self.assertEqual(key.path, _PATH) class Test__pb_attr_value(unittest2.TestCase): diff --git a/gcloud/datastore/test_key.py b/gcloud/datastore/test_key.py index 91a8d96b5863a..c5922241065ff 100644 --- a/gcloud/datastore/test_key.py +++ b/gcloud/datastore/test_key.py @@ -26,10 +26,10 @@ def _makeOne(self, path=None, namespace=None, dataset_id=None): def test_ctor_defaults(self): key = self._makeOne() - self.assertEqual(key._dataset_id, None) - self.assertEqual(key.namespace(), None) - self.assertEqual(key.kind(), '') - self.assertEqual(key.path(), [{'kind': ''}]) + self.assertEqual(key.dataset_id, None) + self.assertEqual(key.namespace, None) + self.assertEqual(key.kind, '') + self.assertEqual(key.path, [{'kind': ''}]) def test_ctor_explicit(self): _DATASET = 'DATASET' @@ -38,10 +38,10 @@ def test_ctor_explicit(self): _ID = 1234 _PATH = [{'kind': _KIND, 'id': _ID}] key = self._makeOne(_PATH, _NAMESPACE, _DATASET) - self.assertEqual(key._dataset_id, _DATASET) - self.assertEqual(key.namespace(), _NAMESPACE) - self.assertEqual(key.kind(), _KIND) - self.assertEqual(key.path(), _PATH) + self.assertEqual(key.dataset_id, _DATASET) + self.assertEqual(key.namespace, _NAMESPACE) + self.assertEqual(key.kind, _KIND) + self.assertEqual(key.path, _PATH) def test__clone(self): _DATASET = 'DATASET' @@ -51,10 +51,10 @@ def test__clone(self): _PATH = [{'kind': _KIND, 'id': _ID}] key = self._makeOne(_PATH, _NAMESPACE, _DATASET) clone = key._clone() - self.assertEqual(clone._dataset_id, _DATASET) - self.assertEqual(clone.namespace(), _NAMESPACE) - self.assertEqual(clone.kind(), _KIND) - self.assertEqual(clone.path(), _PATH) + self.assertEqual(clone.dataset_id, _DATASET) + self.assertEqual(clone.namespace, _NAMESPACE) + self.assertEqual(clone.kind, _KIND) + self.assertEqual(clone.path, _PATH) def test_to_protobuf_defaults(self): from gcloud.datastore.datastore_v1_pb2 import Key as KeyPB @@ -104,115 +104,42 @@ def test_to_protobuf_w_explicit_path(self): def test_is_partial_no_name_or_id(self): key = self._makeOne() - self.assertTrue(key.is_partial()) + self.assertTrue(key.is_partial) def test_is_partial_w_id(self): _KIND = 'KIND' _ID = 1234 _PATH = [{'kind': _KIND, 'id': _ID}] key = self._makeOne(path=_PATH) - self.assertFalse(key.is_partial()) + self.assertFalse(key.is_partial) def test_is_partial_w_name(self): _KIND = 'KIND' _NAME = 'NAME' _PATH = [{'kind': _KIND, 'name': _NAME}] key = self._makeOne(path=_PATH) - self.assertFalse(key.is_partial()) - - def test_namespace_setter(self): - _DATASET = 'DATASET' - _NAMESPACE = 'NAMESPACE' - _KIND = 'KIND' - _NAME = 'NAME' - _PATH = [{'kind': _KIND, 'name': _NAME}] - key = self._makeOne(path=_PATH, dataset_id=_DATASET) - after = key.namespace(_NAMESPACE) - self.assertFalse(after is key) - self.assertTrue(isinstance(after, self._getTargetClass())) - self.assertEqual(after._dataset_id, _DATASET) - self.assertEqual(after.namespace(), _NAMESPACE) - self.assertEqual(after.path(), _PATH) - - def test_path_setter(self): - _DATASET = 'DATASET' - _NAMESPACE = 'NAMESPACE' - _KIND = 'KIND' - _NAME = 'NAME' - _PATH = [{'kind': _KIND, 'name': _NAME}] - key = self._makeOne(namespace=_NAMESPACE, dataset_id=_DATASET) - after = key.path(_PATH) - self.assertFalse(after is key) - self.assertTrue(isinstance(after, self._getTargetClass())) - self.assertEqual(after._dataset_id, _DATASET) - self.assertEqual(after.namespace(), _NAMESPACE) - self.assertEqual(after.path(), _PATH) + self.assertFalse(key.is_partial) def test_kind_getter_empty_path(self): _DATASET = 'DATASET' _NAMESPACE = 'NAMESPACE' key = self._makeOne(namespace=_NAMESPACE, dataset_id=_DATASET) key._path = () # edge case - self.assertEqual(key.kind(), None) - - def test_kind_setter(self): - _DATASET = 'DATASET' - _NAMESPACE = 'NAMESPACE' - _KIND_BEFORE = 'KIND_BEFORE' - _KIND_AFTER = 'KIND_AFTER' - _NAME = 'NAME' - _PATH = [{'kind': _KIND_BEFORE, 'name': _NAME}] - key = self._makeOne(_PATH, _NAMESPACE, _DATASET) - after = key.kind(_KIND_AFTER) - self.assertFalse(after is key) - self.assertTrue(isinstance(after, self._getTargetClass())) - self.assertEqual(after._dataset_id, _DATASET) - self.assertEqual(after.namespace(), _NAMESPACE) - self.assertEqual(after.path(), [{'kind': _KIND_AFTER, 'name': _NAME}]) + self.assertEqual(key.kind, None) def test_id_getter_empty_path(self): key = self._makeOne() key._path = () # edge case - self.assertEqual(key.id(), None) - - def test_id_setter(self): - _DATASET = 'DATASET' - _NAMESPACE = 'NAMESPACE' - _KIND = 'KIND' - _ID_BEFORE = 1234 - _ID_AFTER = 5678 - _PATH = [{'kind': _KIND, 'id': _ID_BEFORE}] - key = self._makeOne(_PATH, _NAMESPACE, _DATASET) - after = key.id(_ID_AFTER) - self.assertFalse(after is key) - self.assertTrue(isinstance(after, self._getTargetClass())) - self.assertEqual(after._dataset_id, _DATASET) - self.assertEqual(after.namespace(), _NAMESPACE) - self.assertEqual(after.path(), [{'kind': _KIND, 'id': _ID_AFTER}]) + self.assertEqual(key.id, None) def test_name_getter_empty_path(self): key = self._makeOne() key._path = () # edge case - self.assertEqual(key.name(), None) - - def test_name_setter(self): - _DATASET = 'DATASET' - _NAMESPACE = 'NAMESPACE' - _KIND = 'KIND' - _NAME_BEFORE = 'NAME_BEFORE' - _NAME_AFTER = 'NAME_AFTER' - _PATH = [{'kind': _KIND, 'name': _NAME_BEFORE}] - key = self._makeOne(_PATH, _NAMESPACE, _DATASET) - after = key.name(_NAME_AFTER) - self.assertFalse(after is key) - self.assertTrue(isinstance(after, self._getTargetClass())) - self.assertEqual(after._dataset_id, _DATASET) - self.assertEqual(after.namespace(), _NAMESPACE) - self.assertEqual(after.path(), [{'kind': _KIND, 'name': _NAME_AFTER}]) + self.assertEqual(key.name, None) def test_id_or_name_no_name_or_id(self): key = self._makeOne() - self.assertEqual(key.id_or_name(), None) + self.assertEqual(key.id_or_name, None) def test_id_or_name_no_name_or_id_child(self): _KIND = 'KIND' @@ -220,14 +147,14 @@ def test_id_or_name_no_name_or_id_child(self): _ID = 5678 _PATH = [{'kind': _KIND, 'id': _ID, 'name': _NAME}, {'kind': ''}] key = self._makeOne(path=_PATH) - self.assertEqual(key.id_or_name(), None) + self.assertEqual(key.id_or_name, None) def test_id_or_name_w_id_only(self): _KIND = 'KIND' _ID = 1234 _PATH = [{'kind': _KIND, 'id': _ID}] key = self._makeOne(path=_PATH) - self.assertEqual(key.id_or_name(), _ID) + self.assertEqual(key.id_or_name, _ID) def test_id_or_name_w_id_and_name(self): _KIND = 'KIND' @@ -235,22 +162,22 @@ def test_id_or_name_w_id_and_name(self): _NAME = 'NAME' _PATH = [{'kind': _KIND, 'id': _ID, 'name': _NAME}] key = self._makeOne(path=_PATH) - self.assertEqual(key.id_or_name(), _ID) + self.assertEqual(key.id_or_name, _ID) def test_id_or_name_w_name_only(self): _KIND = 'KIND' _NAME = 'NAME' _PATH = [{'kind': _KIND, 'name': _NAME}] key = self._makeOne(path=_PATH) - self.assertEqual(key.id_or_name(), _NAME) + self.assertEqual(key.id_or_name, _NAME) def test_parent_default(self): key = self._makeOne() - self.assertEqual(key.parent(), None) + self.assertEqual(key.parent, None) def test_parent_explicit_top_level(self): key = self._makeOne(path=[{'kind': 'abc', 'name': 'def'}]) - self.assertEqual(key.parent(), None) + self.assertEqual(key.parent, None) def test_parent_explicit_nested(self): parent_part = {'kind': 'abc', 'name': 'def'} @@ -258,4 +185,4 @@ def test_parent_explicit_nested(self): parent_part, {'kind': 'ghi', 'id': 123}, ]) - self.assertEqual(key.parent().path(), [parent_part]) + self.assertEqual(key.parent.path, [parent_part]) diff --git a/gcloud/datastore/test_query.py b/gcloud/datastore/test_query.py index a813820ed3ac7..d7cfe83dd64e6 100644 --- a/gcloud/datastore/test_query.py +++ b/gcloud/datastore/test_query.py @@ -330,7 +330,7 @@ def _fetch_page_helper(self, cursor=b'\x00', limit=None, self.assertEqual(more_results, _MORE_RESULTS) self.assertEqual(len(entities), 1) - self.assertEqual(entities[0].key().path(), + self.assertEqual(entities[0].key().path, [{'kind': _KIND, 'id': _ID}]) limited_query = query if limit is not None: diff --git a/gcloud/datastore/test_transaction.py b/gcloud/datastore/test_transaction.py index b849a74b140bc..602aca25d9f60 100644 --- a/gcloud/datastore/test_transaction.py +++ b/gcloud/datastore/test_transaction.py @@ -214,9 +214,8 @@ def __init__(self, *new_keys): class _Key(object): _path = None - def path(self, path): - self._path = path - return self + def _clone(self): + return _Key() class _Entity(object): diff --git a/gcloud/datastore/transaction.py b/gcloud/datastore/transaction.py index 6ae0846a83523..3c9473bdd91fe 100644 --- a/gcloud/datastore/transaction.py +++ b/gcloud/datastore/transaction.py @@ -63,8 +63,8 @@ class Transaction(_implicit_environ._DatastoreBase): >>> with dataset.transaction(): ... entity = dataset.entity('Thing') ... entity.save() - ... assert entity.key().is_partial() # There is no ID on this key. - >>> assert not entity.key().is_partial() # There *is* an ID. + ... assert entity.key().is_partial # There is no ID on this key. + >>> assert not entity.key().is_partial # There *is* an ID. .. warning:: If you're using the automatically generated ID functionality, it's important that you only use @@ -235,7 +235,10 @@ def commit(self): for i, entity in enumerate(self._auto_id_entities): key_pb = result.insert_auto_id_key[i] key = helpers.key_from_protobuf(key_pb) - entity.key(entity.key().path(key.path())) + # This is a temporary hack. Will be addressed in 451 #6. + new_key = entity.key()._clone() + new_key._path = key.path + entity.key(key) # Tell the connection that the transaction is over. self.connection().transaction(None) diff --git a/regression/clear_datastore.py b/regression/clear_datastore.py index ba3efedd55da2..ecc75ada54b4a 100644 --- a/regression/clear_datastore.py +++ b/regression/clear_datastore.py @@ -45,7 +45,7 @@ def fetch_keys(dataset, kind, fetch_max=FETCH_MAX, query=None, cursor=None): def get_ancestors(entities): # NOTE: A key will always have at least one path element. - key_roots = [entity.key().path()[0] for entity in entities] + key_roots = [entity.key().path[0] for entity in entities] # Turn into hashable type so we can use set to get unique roots. # Also sorted the items() to ensure uniqueness. key_roots = [tuple(sorted(root.items())) for root in key_roots] diff --git a/regression/datastore.py b/regression/datastore.py index a7fd13b27a647..8e4822a7b5854 100644 --- a/regression/datastore.py +++ b/regression/datastore.py @@ -45,9 +45,9 @@ def test_allocate_ids(self): unique_ids = set() for key in allocated_keys: - unique_ids.add(key.id()) - self.assertEqual(key.name(), None) - self.assertNotEqual(key.id(), None) + unique_ids.add(key.id) + self.assertEqual(key.name, None) + self.assertNotEqual(key.id, None) self.assertEqual(len(unique_ids), num_ids) @@ -71,9 +71,9 @@ def _get_post(self, name=None, key_id=None, post_content=None): # Update the entity key. key = None if name is not None: - key = entity.key().name(name) + key = datastore.key.Key(path=[{'kind': 'Post', 'name': name}]) if key_id is not None: - key = entity.key().id(key_id) + key = datastore.key.Key(path=[{'kind': 'Post', 'id': key_id}]) if key is not None: entity.key(key) @@ -87,14 +87,14 @@ def _generic_test_post(self, name=None, key_id=None): self.case_entities_to_delete.append(entity) if name is not None: - self.assertEqual(entity.key().name(), name) + self.assertEqual(entity.key().name, name) if key_id is not None: - self.assertEqual(entity.key().id(), key_id) + self.assertEqual(entity.key().id, key_id) retrieved_entity = datastore.get_entity(entity.key()) # Check the keys are the same. - self.assertEqual(retrieved_entity.key().path(), entity.key().path()) - self.assertEqual(retrieved_entity.key().namespace(), - entity.key().namespace()) + self.assertEqual(retrieved_entity.key().path, entity.key().path) + self.assertEqual(retrieved_entity.key().namespace, + entity.key().namespace) # Check the data is the same. self.assertEqual(retrieved_entity.to_dict(), entity.to_dict()) @@ -157,8 +157,8 @@ def test_save_key_self_reference(self): stored_person = stored_persons[0] self.assertEqual(stored_person['fullName'], entity['fullName']) - self.assertEqual(stored_person.key().path(), key.path()) - self.assertEqual(stored_person.key().namespace(), key.namespace()) + self.assertEqual(stored_person.key().path, key.path) + self.assertEqual(stored_person.key().namespace, key.namespace) class TestDatastoreQuery(TestDatastore): @@ -261,12 +261,12 @@ def test_projection_query(self): # Check both Catelyn keys are the same. catelyn_stark_key = catelyn_stark_entity.key() catelyn_tully_key = catelyn_tully_entity.key() - self.assertEqual(catelyn_stark_key.path(), catelyn_tully_key.path()) - self.assertEqual(catelyn_stark_key.namespace(), - catelyn_tully_key.namespace()) - # Also check the _dataset_id since both retrieved from datastore. - self.assertEqual(catelyn_stark_key._dataset_id, - catelyn_tully_key._dataset_id) + self.assertEqual(catelyn_stark_key.path, catelyn_tully_key.path) + self.assertEqual(catelyn_stark_key.namespace, + catelyn_tully_key.namespace) + # Also check the dataset_id since both retrieved from datastore. + self.assertEqual(catelyn_stark_key.dataset_id, + catelyn_tully_key.dataset_id) sansa_entity = entities[8] self.assertEqual(sansa_entity.to_dict(),