Merge "Adjust some parameters and return types"

openstack/object_store/v1/_proxy.py

@@ -19,34 +19,40 @@ class Proxy(object):
     def __init__(self, session):
         self.session = session
 
-    def get_account_metadata(self, container=None):
+    def get_account_metadata(self):
         """Get metatdata for this account.
 
-        :param container: The container to retreive metadata for.
+        :rtype:
-        :type container:
             :class:`~openstack.object_store.v1.container.Container`
         """
-        # TODO(briancurtin): should just use Container.head directly?
+        return _container.Container().head(self.session)
-        if container is None:
-            container = _container.Container()
-        container.head(self.session)
-        return container
 
     def set_account_metadata(self, container):
         """Set metatdata for this account.
 
-        :param container: The container to set metadata for.
+        :param container: Account metadata specified on a
+            :class:`~openstack.object_store.v1.container.Container` object
+            to be sent to the server.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype: ``None``
         """
         container.update(self.session)
-        return container
 
     def containers(self, limit=None, marker=None, **kwargs):
-        """Return a generator that yields the account's Container objects.
+        """Obtain Container objects for this account.
 
-        :param int limit: Set the limit of how many containers to retrieve.
+        :param int limit: Set the limit of how many containers to retrieve
+            in each request to the server. By default, the value is ``None``,
+            retrieving the maximum amount of containers per request that
+            your server allows.
         :param str marker: The name of the container to begin iterating from.
+            By default, the value is ``None``, returning all available
+            containers.
+
+        :rtype: A generator of
+            :class:`~openstack.object_store.v1.container.Container` objects.
         """
         return _container.Container.list(self.session, limit=limit,
                                          marker=marker, **kwargs)
@@ -54,44 +60,58 @@ class Proxy(object):
     def get_container_metadata(self, container):
         """Get metatdata for a container.
 
-        :param container: The container to retreive metadata for.
+        :param container: The container to retreive metadata for. You can
+            pass a container object or the name of a container to
+            retrieve metadata for.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype:
+            :class:`~openstack.object_store.v1.container.Container`
+        :raises: :exc:`ValueError` when an unnamed container object was
+            specified, as this would instead retrieve account metadata.
         """
         container = _container.Container.from_id(container)
-        # TODO(briancurtin): may want to check if the container has a
+        if getattr(container, "name") is None:
-        # name at this point. If it doesn't, this call will work but it's
+            msg = "A named container or a name itself must be passed"
-        # actually getting *account* metadata.
+            raise ValueError(msg)
-        container.head(self.session)
+
-        return container
+        return container.head(self.session)
 
     def set_container_metadata(self, container):
         """Set metatdata for a container.
 
-        :param container: The container to set metadata for.
+        :param container: A container object containing metadata to be set.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype: ``None``
         """
         container.create(self.session)
-        return container
 
     def create_container(self, container):
         """Create a container,
 
-        :param container: A container name or object.
+        :param container: The container to create. You can pass a container
+            object or the name of a container to create.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype:
+            :class:`~openstack.object_store.v1.container.Container`
         """
         container = _container.Container.from_id(container)
-        container.create(self.session)
+        return container.create(self.session)
-        return container
 
     def delete_container(self, container):
         """Delete a container.
 
-        :param container: A container name or object.
+        :param container: The container to delete. You can pass a container
+            object or the name of a container to delete.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype: ``None``
         """
         container = _container.Container.from_id(container)
         container.delete(self.session)
@@ -99,9 +119,13 @@ class Proxy(object):
     def objects(self, container, limit=None, marker=None, **kwargs):
         """Return a generator that yields the Container's objects.
 
-        :param container: A container name or object.
+        :param container: A container object or the name of a container
+            that you want to retrieve objects from.
         :type container:
             :class:`~openstack.object_store.v1.container.Container`
+
+        :rtype: A generator of
+            :class:`~openstack.object_store.v1.obj.Object` objects.
         """
         container = _container.Container.from_id(container)
 
@@ -151,8 +175,7 @@ class Proxy(object):
             cnt = _container.Container.from_id(container)
             obj.container = cnt.name
 
-        obj.create(self.session, data)
+        return obj.create(self.session, data)
-        return obj
 
     def copy_object(self):
         """Copy an object."""
@@ -163,6 +186,8 @@ class Proxy(object):
 
         :param obj: The object to delete.
         :type obj: :class:`~openstack.object_store.v1.obj.Object`
+
+        :rtype: ``None``
         """
         obj.delete(self.session)
 
@@ -171,15 +196,18 @@ class Proxy(object):
 
         :param obj: The object to retreive metadata from.
         :type obj: :class:`~openstack.object_store.v1.obj.Object`
+
+        :return: A :class:`~openstack.object_store.v1.obj.Object`
+            populated with the server's response.
         """
-        obj.head(self.session)
+        return obj.head(self.session)
-        return obj
 
     def set_object_metadata(self, obj):
         """Set metatdata for an object.
 
         :param obj: The object to set metadata for.
         :type obj: :class:`~openstack.object_store.v1.obj.Object`
+
+        :rtype: ``None``
         """
         obj.create(self.session)
-        return obj

openstack/tests/object_store/v1/test_proxy.py

@@ -35,20 +35,25 @@ class TestObjectStoreProxy(test_proxy_base.TestProxyBase):
 
 class Test_account_metadata(TestObjectStoreProxy):
 
-    def _test_container_object(self, method, verb):
+    @mock.patch("openstack.resource.Resource")
-        container = mock.MagicMock()
+    def test_get_account_metadata(self, mock_resource):
+        cont = container.Container()
+        mock_resource = mock.MagicMock()
+        mock_resource.head.return_value = cont
 
-        result = method(container)
+        result = self.proxy.get_account_metadata()
 
-        self.assertIs(result, container)
+        self.assertEqual(result, cont)
-
-        getattr(container, verb).assert_called_once_with(self.session)
-
-    def test_get_account_metadata(self):
-        self._test_container_object(self.proxy.get_account_metadata, "head")
 
     def test_set_account_metadata(self):
-        self._test_container_object(self.proxy.set_account_metadata, "update")
+        container = mock.MagicMock()
+        container.update.return_value = container
+
+        result = self.proxy.set_account_metadata(container)
+
+        self.assertIs(result, None)
+
+        container.update.assert_called_once_with(self.session)
 
     @mock.patch("openstack.object_store.v1._proxy._container.Container")
     def test_get_account_metadata_no_arg(self, mock_container):
@@ -146,6 +151,8 @@ class Test_container_metadata(TestObjectStoreProxy):
     @mock.patch("openstack.resource.Resource.from_id")
     def test_get_container_metadata_object(self, mock_fi):
         container = mock.MagicMock()
+        container.name = "test"
+        container.head.return_value = container
         mock_fi.return_value = container
 
         result = self.proxy.get_container_metadata(container)
@@ -158,6 +165,7 @@ class Test_container_metadata(TestObjectStoreProxy):
         name = six.text_type("my_container")
         created_container = mock.MagicMock()
         created_container.name = name
+        created_container.head.return_value = created_container
         mock_fi.return_value = created_container
 
         result = self.proxy.get_container_metadata(name)
@@ -170,7 +178,7 @@ class Test_container_metadata(TestObjectStoreProxy):
 
         result = self.proxy.set_container_metadata(container)
 
-        self.assertIs(result, container)
+        self.assertIsNone(result)
         container.create.assert_called_once_with(self.session)
 
 
@@ -179,6 +187,7 @@ class Test_create_container(TestObjectStoreProxy):
     @mock.patch("openstack.resource.Resource.from_id")
     def test_container_object(self, mock_fi):
         container = mock.MagicMock()
+        container.create.return_value = container
         mock_fi.return_value = container
 
         result = self.proxy.create_container(container)
@@ -191,6 +200,7 @@ class Test_create_container(TestObjectStoreProxy):
         name = six.text_type("my_container")
         created_container = mock.MagicMock()
         created_container.name = name
+        created_container.create.return_value = created_container
         mock_fi.return_value = created_container
 
         result = self.proxy.create_container(name)
@@ -362,6 +372,7 @@ class Test_create_object(TestObjectStoreProxy):
     def test_create_with_obj_name_real_container(self, mock_fi):
         created_object = mock.MagicMock()
         created_object.name = self.object_name
+        created_object.create.return_value = created_object
         # Since we're using a MagicMock, we have to explicitly set this to
         # None otherwise when it gets accessed it'll have a value which
         # is not what we want to happen.
@@ -382,6 +393,7 @@ class Test_create_object(TestObjectStoreProxy):
     def test_create_with_real_obj_real_container(self):
         ob = obj.Object.new(name=self.object_name)
         ob.create = mock.MagicMock()
+        ob.create.return_value = ob
         cont = container.Container.new(name=self.container_name)
 
         result = self.proxy.create_object(self.the_data, ob, cont)
@@ -395,6 +407,7 @@ class Test_create_object(TestObjectStoreProxy):
         ob = obj.Object.new(name=self.object_name,
                             container=self.container_name)
         ob.create = mock.MagicMock()
+        ob.create.return_value = ob
 
         result = self.proxy.create_object(self.the_data, ob)
 
@@ -409,6 +422,7 @@ class Test_object_metadata(TestObjectStoreProxy):
     @mock.patch("openstack.resource.Resource.from_id")
     def test_get_object_metadata(self, mock_fi):
         ob = mock.MagicMock()
+        ob.head.return_value = ob
         mock_fi.return_value = ob
 
         result = self.proxy.get_object_metadata(ob)
@@ -421,7 +435,7 @@ class Test_object_metadata(TestObjectStoreProxy):
 
         result = self.proxy.set_object_metadata(ob)
 
-        self.assertIs(result, ob)
+        self.assertIsNone(result)
         ob.create.assert_called_once_with(self.session)