Rework the manager and object classes

Add new RESTObject and RESTManager base class, linked to a bunch of
Mixin class to implement the actual CRUD methods.

Object are generated by the managers, and special cases are handled in
the derivated classes.

Both ways (old and new) can be used together, migrate only a few v4
objects to the new method as a POC.

TODO: handle managers on generated objects (have to deal with attributes
in the URLs).

3 files changed, 399 insertions, 106 deletions

diff --git a/gitlab/__init__.py b/gitlab/__init__.py
index dbb7f85..d27fcf7 100644
--- a/gitlab/__init__.py
+++ b/gitlab/__init__.py
@@ -644,9 +644,12 @@ class Gitlab(object):
         opts = self._get_session_opts(content_type='application/json')
         result = self.session.request(verb, url, json=post_data,
                                       params=params, stream=streamed, **opts)
-        if not (200 <= result.status_code < 300):
-            raise GitlabHttpError(response_code=result.status_code)
-        return result
+        if 200 <= result.status_code < 300:
+            return result
+
+
+        raise GitlabHttpError(response_code=result.status_code,
+                              error_message=result.content)
 
     def http_get(self, path, query_data={}, streamed=False, **kwargs):
         """Make a GET request to the Gitlab server.
@@ -748,7 +751,7 @@ class Gitlab(object):
             GitlabHttpError: When the return code is not 2xx
             GitlabParsingError: IF the json data could not be parsed
         """
-        result = self.hhtp_request('put', path, query_data=query_data,
+        result = self.http_request('put', path, query_data=query_data,
                                    post_data=post_data, **kwargs)
         try:
             return result.json()
@@ -808,6 +811,9 @@ class GitlabList(object):
     def __iter__(self):
         return self
 
+    def __len__(self):
+        return self._total_pages
+
     def __next__(self):
         return self.next()
 
@@ -819,6 +825,6 @@ class GitlabList(object):
         except IndexError:
             if self._next_url:
                 self._query(self._next_url)
-                return self._data[self._current]
+                return self.next()
 
             raise StopIteration
diff --git a/gitlab/base.py b/gitlab/base.py
index 0d82cf1..2e26c64 100644
--- a/gitlab/base.py
+++ b/gitlab/base.py
@@ -531,3 +531,317 @@ class GitlabObject(object):
 
     def __ne__(self, other):
         return not self.__eq__(other)
+
+
+class SaveMixin(object):
+    """Mixin for RESTObject's that can be updated."""
+    def save(self, **kwargs):
+        """Saves the changes made to the object to the server.
+
+        Args:
+            **kwargs: Extra option to send to the server (e.g. sudo)
+
+        The object is updated to match what the server returns.
+        """
+        updated_data = {}
+        required, optional = self.manager.get_update_attrs()
+        for attr in required:
+            # Get everything required, no matter if it's been updated
+            updated_data[attr] = getattr(self, attr)
+        # Add the updated attributes
+        updated_data.update(self._updated_attrs)
+
+        # class the manager
+        obj_id = self.get_id()
+        server_data = self.manager.update(obj_id, updated_data, **kwargs)
+        self._updated_attrs = {}
+        self._attrs.update(server_data)
+
+
+class RESTObject(object):
+    """Represents an object built from server data.
+
+    It holds the attributes know from te server, and the updated attributes in
+    another. This allows smart updates, if the object allows it.
+
+    You can redefine ``_id_attr`` in child classes to specify which attribute
+    must be used as uniq ID. None means that the object can be updated without
+    ID in the url.
+    """
+    _id_attr = 'id'
+
+    def __init__(self, manager, attrs):
+        self.__dict__.update({
+            'manager': manager,
+            '_attrs': attrs,
+            '_updated_attrs': {},
+        })
+
+    def __getattr__(self, name):
+        try:
+            return self.__dict__['_updated_attrs'][name]
+        except KeyError:
+            try:
+                return self.__dict__['_attrs'][name]
+            except KeyError:
+                raise AttributeError(name)
+
+    def __setattr__(self, name, value):
+        self.__dict__['_updated_attrs'][name] = value
+
+    def __str__(self):
+        data = self._attrs.copy()
+        data.update(self._updated_attrs)
+        return '%s => %s' % (type(self), data)
+
+    def __repr__(self):
+        if self._id_attr :
+            return '<%s %s:%s>' % (self.__class__.__name__,
+                                   self._id_attr,
+                                   self.get_id())
+        else:
+            return '<%s>' % self.__class__.__name__
+
+    def get_id(self):
+        if self._id_attr is None:
+            return None
+        return getattr(self, self._id_attr)
+
+
+class RESTObjectList(object):
+    """Generator object representing a list of RESTObject's.
+
+    This generator uses the Gitlab pagination system to fetch new data when
+    required.
+
+    Note: you should not instanciate such objects, they are returned by calls
+    to RESTManager.list()
+
+    Args:
+        manager: Manager to attach to the created objects
+        obj_cls: Type of objects to create from the json data
+        _list: A GitlabList object
+    """
+    def __init__(self, manager, obj_cls, _list):
+        self.manager = manager
+        self._obj_cls = obj_cls
+        self._list = _list
+
+    def __iter__(self):
+        return self
+
+    def __len__(self):
+        return len(self._list)
+
+    def __next__(self):
+        return self.next()
+
+    def next(self):
+        data = self._list.next()
+        return self._obj_cls(self.manager, data)
+
+
+class GetMixin(object):
+    def get(self, id, **kwargs):
+        """Retrieve a single object.
+
+        Args:
+            id (int or str): ID of the object to retrieve
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+
+        Returns:
+            object: The generated RESTObject.
+
+        Raises:
+            GitlabGetError: If the server cannot perform the request.
+        """
+        path = '%s/%s' % (self._path, id)
+        server_data = self.gitlab.http_get(path, **kwargs)
+        return self._obj_cls(self, server_data)
+
+
+class GetWithoutIdMixin(object):
+    def get(self, **kwargs):
+        """Retrieve a single object.
+
+        Args:
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+
+        Returns:
+            object: The generated RESTObject.
+
+        Raises:
+            GitlabGetError: If the server cannot perform the request.
+        """
+        server_data = self.gitlab.http_get(self._path, **kwargs)
+        return self._obj_cls(self, server_data)
+
+
+class ListMixin(object):
+    def list(self, **kwargs):
+        """Retrieves a list of objects.
+
+        Args:
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo).
+                      If ``all`` is passed and set to True, the entire list of
+                      objects will be returned.
+
+        Returns:
+            RESTObjectList: Generator going through the list of objects, making
+                            queries to the server when required.
+                            If ``all=True`` is passed as argument, returns
+                            list(RESTObjectList).
+        """
+
+        obj = self.gitlab.http_list(self._path, **kwargs)
+        if isinstance(obj, list):
+            return [self._obj_cls(self, item) for item in obj]
+        else:
+            return RESTObjectList(self, self._obj_cls, obj)
+
+
+class GetFromListMixin(ListMixin):
+    def get(self, id, **kwargs):
+        """Retrieve a single object.
+
+        Args:
+            id (int or str): ID of the object to retrieve
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+
+        Returns:
+            object: The generated RESTObject.
+
+        Raises:
+            GitlabGetError: If the server cannot perform the request.
+        """
+        gen = self.list()
+        for obj in gen:
+            if str(obj.get_id()) == str(id):
+                return obj
+
+
+class RetrieveMixin(ListMixin, GetMixin):
+    pass
+
+
+class CreateMixin(object):
+    def _check_missing_attrs(self, data):
+        required, optional = self.get_create_attrs()
+        missing = []
+        for attr in required:
+            if attr not in data:
+                missing.append(attr)
+                continue
+        if missing:
+            raise AttributeError("Missing attributes: %s" % ", ".join(missing))
+
+    def get_create_attrs(self):
+        """Returns the required and optional arguments.
+
+        Returns:
+            tuple: 2 items: list of required arguments and list of optional
+                   arguments for creation (in that order)
+        """
+        if hasattr(self, '_create_attrs'):
+            return (self._create_attrs['required'],
+                    self._create_attrs['optional'])
+        return (tuple(), tuple())
+
+    def create(self, data, **kwargs):
+        """Created a new object.
+
+        Args:
+            data (dict): parameters to send to the server to create the
+                         resource
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+
+        Returns:
+            RESTObject: a new instance of the manage object class build with
+                        the data sent by the server
+        """
+        self._check_missing_attrs(data)
+        if hasattr(self, '_sanitize_data'):
+            data = self._sanitize_data(data, 'create')
+        server_data = self.gitlab.http_post(self._path, post_data=data, **kwargs)
+        return self._obj_cls(self, server_data)
+
+
+class UpdateMixin(object):
+    def _check_missing_attrs(self, data):
+        required, optional = self.get_update_attrs()
+        missing = []
+        for attr in required:
+            if attr not in data:
+                missing.append(attr)
+                continue
+        if missing:
+            raise AttributeError("Missing attributes: %s" % ", ".join(missing))
+
+    def get_update_attrs(self):
+        """Returns the required and optional arguments.
+
+        Returns:
+            tuple: 2 items: list of required arguments and list of optional
+                   arguments for update (in that order)
+        """
+        if hasattr(self, '_update_attrs'):
+            return (self._update_attrs['required'],
+                    self._update_attrs['optional'])
+        return (tuple(), tuple())
+
+    def update(self, id=None, new_data={}, **kwargs):
+        """Update an object on the server.
+
+        Args:
+            id: ID of the object to update (can be None if not required)
+            new_data: the update data for the object
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+
+        Returns:
+            dict: The new object data (*not* a RESTObject)
+        """
+
+        if id is None:
+            path = self._path
+        else:
+            path = '%s/%s' % (self._path, id)
+
+        self._check_missing_attrs(new_data)
+        if hasattr(self, '_sanitize_data'):
+            data = self._sanitize_data(new_data, 'update')
+        server_data = self.gitlab.http_put(self._path, post_data=data,
+                                           **kwargs)
+        return server_data
+
+
+class DeleteMixin(object):
+    def delete(self, id, **kwargs):
+        """Deletes an object on the server.
+
+        Args:
+            id: ID of the object to delete
+            **kwargs: Extra data to send to the Gitlab server (e.g. sudo)
+        """
+        path = '%s/%s' % (self._path, id)
+        self.gitlab.http_delete(path, **kwargs)
+
+
+class CRUDMixin(GetMixin, ListMixin, CreateMixin, UpdateMixin, DeleteMixin):
+    pass
+
+
+class RESTManager(object):
+    """Base class for CRUD operations on objects.
+
+    Derivated class must define ``_path`` and ``_obj_cls``.
+
+    ``_path``: Base URL path on which requests will be sent (e.g. '/projects')
+    ``_obj_cls``: The class of objects that will be created
+    """
+
+    _path = None
+    _obj_cls = None
+
+    def __init__(self, gl, parent_attrs={}):
+        self.gitlab = gl
+        self._parent_attrs = {}  # for nested managers
diff --git a/gitlab/v4/objects.py b/gitlab/v4/objects.py
index 6283149..9e2574e 100644
--- a/gitlab/v4/objects.py
+++ b/gitlab/v4/objects.py
@@ -40,7 +40,7 @@ ACCESS_MASTER = 40
 ACCESS_OWNER = 50
 
 
-class SidekiqManager(object):
+class SidekiqManager(RESTManager):
     """Manager for the Sidekiq methods.
 
     This manager doesn't actually manage objects but provides helper fonction
@@ -212,133 +212,106 @@ class CurrentUser(GitlabObject):
     )
 
 
-class ApplicationSettings(GitlabObject):
-    _url = '/application/settings'
-    _id_in_update_url = False
-    getRequiresId = False
-    optionalUpdateAttrs = ['after_sign_out_path',
-                           'container_registry_token_expire_delay',
-                           'default_branch_protection',
-                           'default_project_visibility',
-                           'default_projects_limit',
-                           'default_snippet_visibility',
-                           'domain_blacklist',
-                           'domain_blacklist_enabled',
-                           'domain_whitelist',
-                           'enabled_git_access_protocol',
-                           'gravatar_enabled',
-                           'home_page_url',
-                           'max_attachment_size',
-                           'repository_storage',
-                           'restricted_signup_domains',
-                           'restricted_visibility_levels',
-                           'session_expire_delay',
-                           'sign_in_text',
-                           'signin_enabled',
-                           'signup_enabled',
-                           'twitter_sharing_enabled',
-                           'user_oauth_applications']
-    canList = False
-    canCreate = False
-    canDelete = False
+class ApplicationSettings(SaveMixin, RESTObject):
+    _id_attr = None
+
+
+class ApplicationSettingsManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
+    _path = '/application/settings'
+    _obj_cls = ApplicationSettings
+    _update_attrs = {
+        'required': tuple(),
+        'optional': ('after_sign_out_path',
+                     'container_registry_token_expire_delay',
+                     'default_branch_protection', 'default_project_visibility',
+                     'default_projects_limit', 'default_snippet_visibility',
+                     'domain_blacklist', 'domain_blacklist_enabled',
+                     'domain_whitelist', 'enabled_git_access_protocol',
+                     'gravatar_enabled', 'home_page_url',
+                     'max_attachment_size', 'repository_storage',
+                     'restricted_signup_domains',
+                     'restricted_visibility_levels', 'session_expire_delay',
+                     'sign_in_text', 'signin_enabled', 'signup_enabled',
+                     'twitter_sharing_enabled', 'user_oauth_applications')
+    }
 
-    def _data_for_gitlab(self, extra_parameters={}, update=False,
-                         as_json=True):
-        data = (super(ApplicationSettings, self)
-                ._data_for_gitlab(extra_parameters, update=update,
-                                  as_json=False))
-        if not self.domain_whitelist:
-            data.pop('domain_whitelist', None)
-        return json.dumps(data)
+    def _sanitize_data(self, data, action):
+        new_data = data.copy()
+        if 'domain_whitelist' in data and data['domain_whitelist'] is None:
+            new_data.pop('domain_whitelist')
+        return new_data
 
 
-class ApplicationSettingsManager(BaseManager):
-    obj_cls = ApplicationSettings
+class BroadcastMessage(SaveMixin, RESTObject):
+    pass
 
 
-class BroadcastMessage(GitlabObject):
-    _url = '/broadcast_messages'
-    requiredCreateAttrs = ['message']
-    optionalCreateAttrs = ['starts_at', 'ends_at', 'color', 'font']
-    requiredUpdateAttrs = []
-    optionalUpdateAttrs = ['message', 'starts_at', 'ends_at', 'color', 'font']
+class BroadcastMessageManager(CRUDMixin, RESTManager):
+    _path = '/broadcast_messages'
+    _obj_cls = BroadcastMessage
 
+    _create_attrs = {
+        'required': ('message', ),
+        'optional': ('starts_at', 'ends_at', 'color', 'font'),
+    }
+    _update_attrs = {
+        'required': tuple(),
+        'optional': ('message', 'starts_at', 'ends_at', 'color', 'font'),
+    }
 
-class BroadcastMessageManager(BaseManager):
-    obj_cls = BroadcastMessage
 
+class DeployKey(RESTObject):
+    pass
 
-class DeployKey(GitlabObject):
-    _url = '/deploy_keys'
-    canGet = 'from_list'
-    canCreate = False
-    canUpdate = False
-    canDelete = False
 
+class DeployKeyManager(GetFromListMixin, RESTManager):
+    _path = '/deploy_keys'
+    _obj_cls = DeployKey
 
-class DeployKeyManager(BaseManager):
-    obj_cls = DeployKey
 
+class NotificationSettings(SaveMixin, RESTObject):
+    _id_attr = None
 
-class NotificationSettings(GitlabObject):
-    _url = '/notification_settings'
-    _id_in_update_url = False
-    getRequiresId = False
-    optionalUpdateAttrs = ['level',
-                           'notification_email',
-                           'new_note',
-                           'new_issue',
-                           'reopen_issue',
-                           'close_issue',
-                           'reassign_issue',
-                           'new_merge_request',
-                           'reopen_merge_request',
-                           'close_merge_request',
-                           'reassign_merge_request',
-                           'merge_merge_request']
-    canList = False
-    canCreate = False
-    canDelete = False
 
+class NotificationSettingsManager(GetWithoutIdMixin, UpdateMixin, RESTManager):
+    _path = '/notification_settings'
+    _obj_cls = NotificationSettings
 
-class NotificationSettingsManager(BaseManager):
-    obj_cls = NotificationSettings
+    _update_attrs = {
+        'required': tuple(),
+        'optional': ('level', 'notification_email', 'new_note', 'new_issue',
+                     'reopen_issue', 'close_issue', 'reassign_issue',
+                     'new_merge_request', 'reopen_merge_request',
+                     'close_merge_request', 'reassign_merge_request',
+                     'merge_merge_request')
+    }
 
 
-class Dockerfile(GitlabObject):
-    _url = '/templates/dockerfiles'
-    canDelete = False
-    canUpdate = False
-    canCreate = False
-    idAttr = 'name'
+class Dockerfile(RESTObject):
+    _id_attr = 'name'
 
 
-class DockerfileManager(BaseManager):
-    obj_cls = Dockerfile
+class DockerfileManager(RetrieveMixin, RESTManager):
+    _path = '/templates/dockerfiles'
+    _obj_cls = Dockerfile
 
 
-class Gitignore(GitlabObject):
-    _url = '/templates/gitignores'
-    canDelete = False
-    canUpdate = False
-    canCreate = False
-    idAttr = 'name'
+class Gitignore(RESTObject):
+    _id_attr = 'name'
 
 
-class GitignoreManager(BaseManager):
-    obj_cls = Gitignore
+class GitignoreManager(RetrieveMixin, RESTManager):
+    _path = '/templates/gitignores'
+    _obj_cls = Gitignore
 
 
-class Gitlabciyml(GitlabObject):
-    _url = '/templates/gitlab_ci_ymls'
-    canDelete = False
-    canUpdate = False
-    canCreate = False
-    idAttr = 'name'
+class Gitlabciyml(RESTObject):
+    _id_attr = 'name'
 
 
-class GitlabciymlManager(BaseManager):
-    obj_cls = Gitlabciyml
+class GitlabciymlManager(RetrieveMixin, RESTManager):
+    _path = '/templates/gitlab_ci_ymls'
+    _obj_cls = Gitlabciyml
 
 
 class GroupIssue(GitlabObject):