Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address second part of 451: Remove all setters and make attrs into @property #458

Merged
merged 1 commit into from
Dec 30, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -100,7 +100,7 @@ def __init__(self, dataset=None, kind=None, exclude_from_indexes=()):
# _implicit_environ._DatastoreBase to avoid split MRO.
self._dataset = dataset or _implicit_environ.DATASET
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 @@ -153,7 +153,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 @@ -250,7 +250,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 @@ -266,7 +266,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 @@ -287,7 +290,7 @@ def delete(self):

def __repr__(self):
if self._key:
return '<Entity%s %s>' % (self._key.path(),
return '<Entity%s %s>' % (self._key.path,
super(Entity, self).__repr__())
else:
return '<Entity %s>' % (super(Entity, self).__repr__())
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 @@ -173,4 +173,4 @@ 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))
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(list(result), ['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(list(result), ['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