Merge pull request #458 from dhermes/fix-451-part2

Address second part of 451: Remove all setters and make attrs into @Property

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]

gcloud/datastore/entity.py

@@ -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)
@@ -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.
@@ -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):
@@ -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
 
@@ -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__())

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 '<Key%s>' % self.path()
+        return '<Key%s>' % self.path

gcloud/datastore/test___init__.py

@@ -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))

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(list(result), ['foo'])
         self.assertEqual(result['foo'], 'Foo')
 
@@ -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