Merge pull request #476 from dhermes/fix-451-part8

Address eighth part of 451: Add parent to constructor

Author: tseaver

gcloud/datastore/dataset.py

@@ -120,7 +120,7 @@ def transaction(self, *args, **kwargs):
     def get_entity(self, key):
         """Retrieves entity from the dataset, along with its attributes.
 
-        :type key: :class:`gcloud.datastore.key.Key` or path
+        :type key: :class:`gcloud.datastore.key.Key`
         :param key: The key of the entity to be retrieved.
 
         :rtype: :class:`gcloud.datastore.entity.Entity` or `NoneType`

gcloud/datastore/key.py

@@ -60,12 +60,18 @@ def __init__(self, *path_args, **kwargs):
         :param dataset_id: The dataset ID associated with the key. Required,
                            unless the implicit dataset ID has been set. Can
                            only be passed as a keyword argument.
+
+        :type parent: :class:`gcloud.datastore.key.Key`
+        :param parent: The parent of the key. Can only be passed as a
+                       keyword argument.
         """
-        self._path = self._parse_path(path_args)
         self._flat_path = path_args
-        self._parent = None
+        self._parent = kwargs.get('parent')
         self._namespace = kwargs.get('namespace')
         self._dataset_id = kwargs.get('dataset_id')
+        # _flat_path, _parent, _namespace and _dataset_id must be set before
+        # _combine_args() is called.
+        self._path = self._combine_args()
         self._validate_dataset_id()
 
     def _validate_dataset_id(self):
@@ -87,6 +93,11 @@ def _validate_dataset_id(self):
     def _parse_path(path_args):
         """Parses positional arguments into key path with kinds and IDs.
 
+        :type path_args: :class:`tuple`
+        :param path_args: A tuple from positional arguments. Should be
+                          alternating list of kinds (string) and id/name
+                          parts (int or string).
+
         :rtype: list of dict
         :returns: A list of key parts with kind and id or name set.
         :raises: `ValueError` if there are no `path_args`, if one of the
@@ -123,17 +134,49 @@ def _parse_path(path_args):
 
         return result
 
+    def _combine_args(self):
+        """Sets protected data by combining raw data set from the constructor.
+
+        If a _parent is set, updates the _flat_path and sets the
+        _namespace and _dataset_id if not already set.
+
+        :rtype: list of dict
+        :returns: A list of key parts with kind and id or name set.
+        :raises: `ValueError` if the parent key is not complete.
+        """
+        child_path = self._parse_path(self._flat_path)
+
+        if self._parent is not None:
+            if self._parent.is_partial:
+                raise ValueError('Parent key must be complete.')
+
+            # We know that _parent.path() will return a copy.
+            child_path = self._parent.path + child_path
+            self._flat_path = self._parent.flat_path + self._flat_path
+            if (self._namespace is not None and
+                    self._namespace != self._parent.namespace):
+                raise ValueError('Child namespace must agree with parent\'s.')
+            self._namespace = self._parent.namespace
+            if (self._dataset_id is not None and
+                    self._dataset_id != self._parent.dataset_id):
+                raise ValueError('Child dataset ID must agree with parent\'s.')
+            self._dataset_id = self._parent.dataset_id
+
+        return child_path
+
     def _clone(self):
         """Duplicates the Key.
 
-        We make a shallow copy of the :class:`gcloud.datastore.dataset.Dataset`
-        because it holds a reference an authenticated connection,
-        which we don't want to lose.
+        Most attributes are simple types, so don't require copying. Other
+        attributes like `parent` are long-lived and so we re-use them rather
+        than creating copies.
 
         :rtype: :class:`gcloud.datastore.key.Key`
-        :returns: a new `Key` instance
+        :returns: A new `Key` instance with the same data as the current one.
         """
-        return copy.deepcopy(self)
+        return self.__class__(*self.flat_path, parent=self.parent,
+                              dataset_id=self.dataset_id,
+                              namespace=self.namespace)
 
     def completed_key(self, id_or_name):
         """Creates new key from existing partial key by adding final ID/name.
@@ -285,8 +328,8 @@ def _make_parent(self):
         else:
             parent_args = self.flat_path[:-2]
         if parent_args:
-            return Key(*parent_args, dataset_id=self.dataset_id,
-                       namespace=self.namespace)
+            return self.__class__(*parent_args, dataset_id=self.dataset_id,
+                                  namespace=self.namespace)
 
     @property
     def parent(self):

gcloud/datastore/test_key.py

@@ -41,6 +41,46 @@ def test_ctor_no_dataset(self):
         with _Monkey(_implicit_environ, DATASET=None):
             self.assertRaises(ValueError, klass, 'KIND')
 
+    def test_ctor_parent(self):
+        _PARENT_KIND = 'KIND1'
+        _PARENT_ID = 1234
+        _PARENT_DATASET = 'DATASET-ALT'
+        _PARENT_NAMESPACE = 'NAMESPACE'
+        parent_key = self._makeOne(_PARENT_KIND, _PARENT_ID,
+                                   dataset_id=_PARENT_DATASET,
+                                   namespace=_PARENT_NAMESPACE)
+        _CHILD_KIND = 'KIND2'
+        _CHILD_ID = 2345
+        _PATH = [
+            {'kind': _PARENT_KIND, 'id': _PARENT_ID},
+            {'kind': _CHILD_KIND, 'id': _CHILD_ID},
+        ]
+        key = self._makeOne(_CHILD_KIND, _CHILD_ID, parent=parent_key)
+        self.assertEqual(key.dataset_id, parent_key.dataset_id)
+        self.assertEqual(key.namespace, parent_key.namespace)
+        self.assertEqual(key.kind, _CHILD_KIND)
+        self.assertEqual(key.path, _PATH)
+        self.assertTrue(key.parent is parent_key)
+
+    def test_ctor_partial_parent(self):
+        parent_key = self._makeOne('KIND')
+        with self.assertRaises(ValueError):
+            self._makeOne('KIND2', 1234, parent=parent_key)
+
+    def test_ctor_parent_bad_type(self):
+        with self.assertRaises(AttributeError):
+            self._makeOne('KIND2', 1234, parent=('KIND1', 1234))
+
+    def test_ctor_parent_bad_namespace(self):
+        parent_key = self._makeOne('KIND', 1234, namespace='FOO')
+        with self.assertRaises(ValueError):
+            self._makeOne('KIND2', 1234, namespace='BAR', parent=parent_key)
+
+    def test_ctor_parent_bad_dataset_id(self):
+        parent_key = self._makeOne('KIND', 1234, dataset_id='FOO')
+        with self.assertRaises(ValueError):
+            self._makeOne('KIND2', 1234, dataset_id='BAR', parent=parent_key)
+
     def test_ctor_explicit(self):
         _DATASET = 'DATASET-ALT'
         _NAMESPACE = 'NAMESPACE'