Standardize base.py with novaclient

Main changes:
* deprecate api variable in favour of client
* add documentation
* use to_slug from oslo strutils
* remove Python 2.4 support
* other small fixes from novaclient

Change-Id: Ife54fd3207798ee03101a48bc1cda3b3f62cc5e4

keystoneclient/base.py

@@ -1,5 +1,8 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
 # Copyright 2010 Jacob Kaplan-Moss
-# Copyright 2011 OpenStack LLC.
+# Copyright 2011 OpenStack LLC
+# Copyright 2013 OpenStack Foundation
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -22,23 +25,18 @@ import abc
 import functools
 import urllib
 
+import six
+
 from keystoneclient import exceptions
-
+from keystoneclient.openstack.common import strutils
-
-# Python 2.4 compat
-try:
-    all
-except NameError:
-    def all(iterable):
-        return True not in (not x for x in iterable)
 
 
 def getid(obj):
-    """Abstracts the common pattern of allowing both an object or an object's
+    """Return id if argument is a Resource.
-    ID (UUID) as a parameter when dealing with relationships.
-    """
 
-    # Try to return the object's UUID first, if we have a UUID.
+    Abstracts the common pattern of allowing both an object or an object's ID
+    (UUID) as a parameter when dealing with relationships.
+    """
     try:
         if obj.uuid:
             return obj.uuid
@@ -74,20 +72,42 @@ def filter_kwargs(f):
 
 
 class Manager(object):
-    """Managers interact with a particular type of API (servers, flavors,
+    """Basic manager type providing common operations.
-    images, etc.) and provide CRUD operations for them.
+
+    Managers interact with a particular type of API (servers, flavors, images,
+    etc.) and provide CRUD operations for them.
     """
     resource_class = None
 
-    def __init__(self, api):
+    def __init__(self, client):
-        self.api = api
+        """Initializes Manager with `client`.
+
+        :param client: instance of BaseClient descendant for HTTP requests
+        """
+        super(Manager, self).__init__()
+        self.client = client
+
+    @property
+    def api(self):
+        """Deprecated. Use `client` instead.
+        """
+        return self.client
 
     def _list(self, url, response_key, obj_class=None, body=None):
-        resp = None
+        """List the collection.
+
+        :param url: a partial URL, e.g., '/servers'
+        :param response_key: the key to be looked up in response dictionary,
+            e.g., 'servers'
+        :param obj_class: class for constructing the returned objects
+            (self.resource_class will be used by default)
+        :param body: data that will be encoded as JSON and passed in POST
+            request (GET will be sent by default)
+        """
         if body:
-            resp, body = self.api.post(url, body=body)
+            resp, body = self.client.post(url, body=body)
         else:
-            resp, body = self.api.get(url)
+            resp, body = self.client.get(url)
 
         if obj_class is None:
             obj_class = self.resource_class
@@ -95,38 +115,99 @@ class Manager(object):
         data = body[response_key]
         # NOTE(ja): keystone returns values as list as {'values': [ ... ]}
         #           unlike other services which just return the list...
-        if type(data) is dict:
+        try:
             data = data['values']
+        except (KeyError, TypeError):
+            pass
+
         return [obj_class(self, res, loaded=True) for res in data if res]
 
     def _get(self, url, response_key):
-        resp, body = self.api.get(url)
+        """Get an object from collection.
+
+        :param url: a partial URL, e.g., '/servers'
+        :param response_key: the key to be looked up in response dictionary,
+            e.g., 'server'
+        """
+        resp, body = self.client.get(url)
         return self.resource_class(self, body[response_key], loaded=True)
 
     def _head(self, url):
-        resp, body = self.api.head(url)
+        """Retrieve request headers for an object.
+
+        :param url: a partial URL, e.g., '/servers'
+        """
+        resp, body = self.client.head(url)
         return resp.status_code == 204
 
     def _create(self, url, body, response_key, return_raw=False):
-        resp, body = self.api.post(url, body=body)
+        """Deprecated. Use `_post` instead.
+        """
+        return self._post(url, body, response_key, return_raw)
+
+    def _post(self, url, body, response_key, return_raw=False):
+        """Create an object.
+
+        :param url: a partial URL, e.g., '/servers'
+        :param body: data that will be encoded as JSON and passed in POST
+            request (GET will be sent by default)
+        :param response_key: the key to be looked up in response dictionary,
+            e.g., 'servers'
+        :param return_raw: flag to force returning raw JSON instead of
+            Python object of self.resource_class
+        """
+        resp, body = self.client.post(url, body=body)
         if return_raw:
             return body[response_key]
         return self.resource_class(self, body[response_key])
 
+    def _put(self, url, body=None, response_key=None):
+        """Update an object with PUT method.
+
+        :param url: a partial URL, e.g., '/servers'
+        :param body: data that will be encoded as JSON and passed in POST
+            request (GET will be sent by default)
+        :param response_key: the key to be looked up in response dictionary,
+            e.g., 'servers'
+        """
+        resp, body = self.client.put(url, body=body)
+        # PUT requests may not return a body
+        if body is not None:
+            if response_key is not None:
+                return self.resource_class(self, body[response_key])
+            else:
+                return self.resource_class(self, body)
+
+    def _patch(self, url, body=None, response_key=None):
+        """Update an object with PATCH method.
+
+        :param url: a partial URL, e.g., '/servers'
+        :param body: data that will be encoded as JSON and passed in POST
+            request (GET will be sent by default)
+        :param response_key: the key to be looked up in response dictionary,
+            e.g., 'servers'
+        """
+        resp, body = self.client.patch(url, body=body)
+        if response_key is not None:
+            return self.resource_class(self, body[response_key])
+        else:
+            return self.resource_class(self, body)
+
     def _delete(self, url):
-        resp, body = self.api.delete(url)
+        """Delete an object.
+
+        :param url: a partial URL, e.g., '/servers/my-server'
+        """
+        return self.client.delete(url)
 
     def _update(self, url, body=None, response_key=None, method="PUT",
                 management=True):
-        methods = {"PUT": self.api.put,
+        methods = {"PUT": self.client.put,
-                   "POST": self.api.post,
+                   "POST": self.client.post,
-                   "PATCH": self.api.patch}
+                   "PATCH": self.client.patch}
         try:
-            if body is not None:
             resp, body = methods[method](url, body=body,
                                          management=management)
-            else:
-                resp, body = methods[method](url, management=management)
         except KeyError:
             raise exceptions.ClientException("Invalid update method: %s"
                                              % method)
@@ -136,8 +217,7 @@ class Manager(object):
 
 
 class ManagerWithFind(Manager):
-    """Like a `Manager`, but with additional `find()`/`findall()` methods.
+    """Manager with additional `find()`/`findall()` methods."""
-    """
 
     __metaclass__ = abc.ABCMeta
 
@@ -303,22 +383,38 @@ class CrudManager(Manager):
 
 
 class Resource(object):
-    """A resource represents a particular instance of an object (tenant, user,
+    """Base class for OpenStack resources (tenant, user, etc.).
-    etc). This is pretty much just a bag for attributes.
+
+    This is pretty much just a bag for attributes.
+    """
+
+    HUMAN_ID = False
+    NAME_ATTR = 'name'
+
+    def __init__(self, manager, info, loaded=False):
+        """Populate and bind to a manager.
 
         :param manager: Manager object
         :param info: dictionary representing resource attributes
         :param loaded: prevent lazy-loading if set to True
         """
-    def __init__(self, manager, info, loaded=False):
         self.manager = manager
-        self._info = info
+        self._info = {}
         self._add_details(info)
         self._loaded = loaded
 
+    @property
+    def human_id(self):
+        """Human-readable ID which can be used for bash completion.
+        """
+        if self.NAME_ATTR in self.__dict__ and self.HUMAN_ID:
+            return strutils.to_slug(getattr(self, self.NAME_ATTR))
+        return None
+
     def _add_details(self, info):
-        for (k, v) in info.iteritems():
+        for (k, v) in six.iteritems(info):
             setattr(self, k, v)
+            self._info[k] = v
 
     def __getattr__(self, k):
         if k not in self.__dict__:
@@ -351,6 +447,9 @@ class Resource(object):
         return self.manager.delete(self)
 
     def __eq__(self, other):
+        if not isinstance(other, Resource):
+            return NotImplemented
+        # two resources of different types are not equal
         if not isinstance(other, self.__class__):
             return False
         if hasattr(self, 'id') and hasattr(other, 'id'):

tests/test_base.py

@@ -3,6 +3,10 @@ from keystoneclient.v2_0 import roles
 from tests import utils
 
 
+class HumanReadable(base.Resource):
+    HUMAN_ID = True
+
+
 class BaseTest(utils.TestCase):
 
     def test_resource_repr(self):
@@ -42,3 +46,92 @@ class BaseTest(utils.TestCase):
         r1 = base.Resource(None, {'name': 'joe', 'age': 12})
         r2 = base.Resource(None, {'name': 'joe', 'age': 12})
         self.assertEqual(r1, r2)
+
+        r1 = base.Resource(None, {'id': 1})
+        self.assertNotEqual(r1, object())
+        self.assertNotEqual(r1, {'id': 1})
+
+    def test_human_id(self):
+        r = base.Resource(None, {"name": "1 of !"})
+        self.assertEqual(r.human_id, None)
+        r = HumanReadable(None, {"name": "1 of !"})
+        self.assertEqual(r.human_id, "1-of")
+
+
+class ManagerTest(utils.TestCase):
+    body = {"hello": {"hi": 1}}
+    url = "/test-url"
+
+    def setUp(self):
+        super(ManagerTest, self).setUp()
+        self.mgr = base.Manager(self.client)
+        self.mgr.resource_class = base.Resource
+
+    def test_api(self):
+        self.assertEqual(self.mgr.api, self.client)
+
+    def test_get(self):
+        self.client.get = self.mox.CreateMockAnything()
+        self.client.get(self.url).AndReturn((None, self.body))
+        self.mox.ReplayAll()
+
+        rsrc = self.mgr._get(self.url, "hello")
+        self.assertEqual(rsrc.hi, 1)
+
+    def test_post(self):
+        self.client.post = self.mox.CreateMockAnything()
+        self.client.post(self.url, body=self.body).AndReturn((None, self.body))
+        self.client.post(self.url, body=self.body).AndReturn((None, self.body))
+        self.mox.ReplayAll()
+
+        rsrc = self.mgr._post(self.url, self.body, "hello")
+        self.assertEqual(rsrc.hi, 1)
+
+        rsrc = self.mgr._post(self.url, self.body, "hello", return_raw=True)
+        self.assertEqual(rsrc["hi"], 1)
+
+    def test_put(self):
+        self.client.put = self.mox.CreateMockAnything()
+        self.client.put(self.url, body=self.body).AndReturn((None, self.body))
+        self.client.put(self.url, body=self.body).AndReturn((None, self.body))
+        self.mox.ReplayAll()
+
+        rsrc = self.mgr._put(self.url, self.body, "hello")
+        self.assertEqual(rsrc.hi, 1)
+
+        rsrc = self.mgr._put(self.url, self.body)
+        self.assertEqual(rsrc.hello["hi"], 1)
+
+    def test_patch(self):
+        self.client.patch = self.mox.CreateMockAnything()
+        self.client.patch(self.url, body=self.body).AndReturn(
+            (None, self.body))
+        self.client.patch(self.url, body=self.body).AndReturn(
+            (None, self.body))
+        self.mox.ReplayAll()
+
+        rsrc = self.mgr._patch(self.url, self.body, "hello")
+        self.assertEqual(rsrc.hi, 1)
+
+        rsrc = self.mgr._patch(self.url, self.body)
+        self.assertEqual(rsrc.hello["hi"], 1)
+
+    def test_update(self):
+        self.client.patch = self.mox.CreateMockAnything()
+        self.client.put = self.mox.CreateMockAnything()
+        self.client.patch(
+            self.url, body=self.body, management=False).AndReturn(
+                (None, self.body))
+        self.client.put(self.url, body=None, management=True).AndReturn(
+            (None, self.body))
+        self.mox.ReplayAll()
+
+        rsrc = self.mgr._update(
+            self.url, body=self.body, response_key="hello", method="PATCH",
+            management=False)
+        self.assertEqual(rsrc.hi, 1)
+
+        rsrc = self.mgr._update(
+            self.url, body=None, response_key="hello", method="PUT",
+            management=True)
+        self.assertEqual(rsrc.hi, 1)