Skip to content

Commit

Permalink
Making all attributes on Key read-only and sorts out use of setters.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dhermes committed Dec 23, 2014
1 parent f645323 commit ddaca14
Show file tree
Hide file tree
Showing 13 changed files with 194 additions and 274 deletions.
16 changes: 14 additions & 2 deletions gcloud/datastore/dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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]
13 changes: 8 additions & 5 deletions gcloud/datastore/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand All @@ -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

Expand All @@ -329,6 +332,6 @@ def delete(self):

def __repr__(self):
if self._key:
return '<Entity%s %r>' % (self._key.path(), self._data)
return '<Entity%s %r>' % (self._key.path, self._data)
else:
return '<Entity %r>' % (self._data,)
155 changes: 67 additions & 88 deletions gcloud/datastore/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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 '<Key%s>' % self.path()
return '<Key%s>' % self.path
2 changes: 1 addition & 1 deletion gcloud/datastore/test___init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 7 additions & 16 deletions gcloud/datastore/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -175,37 +175,28 @@ 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'
DATASET = self._makeOne(DATASET_ID, connection=CONNECTION)
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

Expand Down
Loading

0 comments on commit ddaca14

Please sign in to comment.