Adjust some parameters and return types

This change removes a TODO from get_account_metadata which was
previously taking a Container when it wasn't necessary (and didn't make
a ton of sense). Most of the other changes are to document return types
which were previously unlisted, and in a few cases of requests which
don't return a body, their responses are turned to None. If the request
worked, you're going to get an unchanged object back, so no need to be
returning the same thing back at this level.

Change-Id: Ic547e6912e14dfe622533550f1432415070566c1

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)