Implement attribute types to handle special cases

Gauvain Pocentek · Gauvain Pocentek · commit 1940feec3dbb · 2018-03-17T16:46:18.000+01:00
Some attributes need to be parsed/modified to work with the API (for instance lists). This patch provides two attribute types that will simplify parts of the code, and fix some CLI bugs. Fixes #443
diff --git a/docs/cli.rst b/docs/cli.rst
@@ -235,6 +235,12 @@ Use sudo to act as another user (admin only):
 
    $ gitlab project create --name user_project1 --sudo username
 
+List values are comma-separated:
+
+.. code-block:: console
+
+   $ gitlab issue list --labels foo,bar
+
 Reading values from files
 -------------------------
 
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
@@ -108,9 +108,21 @@ def list(self, **kwargs):
             GitlabListError: If the server cannot perform the request
         """
 
+        # Duplicate data to avoid messing with what the user sent us
+        data = kwargs.copy()
+
+        # We get the attributes that need some special transformation
+        types = getattr(self, '_types', {})
+        if types:
+            for attr_name, type_cls in types.items():
+                if attr_name in data.keys():
+                    type_obj = type_cls(data[attr_name])
+                    data[attr_name] = type_obj.get_for_api()
+
         # Allow to overwrite the path, handy for custom listings
-        path = kwargs.pop('path', self.path)
-        obj = self.gitlab.http_list(path, **kwargs)
+        path = data.pop('path', self.path)
+
+        obj = self.gitlab.http_list(path, **data)
         if isinstance(obj, list):
             return [self._obj_cls(self, item) for item in obj]
         else:
@@ -187,8 +199,22 @@ def create(self, data, **kwargs):
             GitlabCreateError: If the server cannot perform the request
         """
         self._check_missing_create_attrs(data)
+
+        # special handling of the object if needed
         if hasattr(self, '_sanitize_data'):
             data = self._sanitize_data(data, 'create')
+
+        # We get the attributes that need some special transformation
+        types = getattr(self, '_types', {})
+
+        if types:
+            # Duplicate data to avoid messing with what the user sent us
+            data = data.copy()
+            for attr_name, type_cls in types.items():
+                if attr_name in data.keys():
+                    type_obj = type_cls(data[attr_name])
+                    data[attr_name] = type_obj.get_for_api()
+
         # Handle specific URL for creation
         path = kwargs.pop('path', self.path)
         server_data = self.gitlab.http_post(path, post_data=data, **kwargs)
@@ -238,11 +264,20 @@ def update(self, id=None, new_data={}, **kwargs):
             path = '%s/%s' % (self.path, id)
 
         self._check_missing_update_attrs(new_data)
+
+        # special handling of the object if needed
         if hasattr(self, '_sanitize_data'):
             data = self._sanitize_data(new_data, 'update')
         else:
             data = new_data
 
+        # We get the attributes that need some special transformation
+        types = getattr(self, '_types', {})
+        for attr_name, type_cls in types.items():
+            if attr_name in data.keys():
+                type_obj = type_cls(data[attr_name])
+                data[attr_name] = type_obj.get_for_api()
+
         return self.gitlab.http_put(path, post_data=data, **kwargs)
 
 
diff --git a/gitlab/tests/test_types.py b/gitlab/tests/test_types.py
@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2018 Gauvain Pocentek <gauvain@pocentek.net>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+try:
+    import unittest
+except ImportError:
+    import unittest2 as unittest
+
+from gitlab import types
+
+
+class TestGitlabAttribute(unittest.TestCase):
+    def test_all(self):
+        o = types.GitlabAttribute('whatever')
+        self.assertEqual('whatever', o.get())
+
+        o.set_from_cli('whatever2')
+        self.assertEqual('whatever2', o.get())
+
+        self.assertEqual('whatever2', o.get_for_api())
+
+        o = types.GitlabAttribute()
+        self.assertEqual(None, o._value)
+
+
+class TestListAttribute(unittest.TestCase):
+    def test_list_input(self):
+        o = types.ListAttribute()
+        o.set_from_cli('foo,bar,baz')
+        self.assertEqual(['foo', 'bar', 'baz'], o.get())
+
+        o.set_from_cli('foo')
+        self.assertEqual(['foo'], o.get())
+
+    def test_empty_input(self):
+        o = types.ListAttribute()
+        o.set_from_cli('')
+        self.assertEqual([], o.get())
+
+        o.set_from_cli('  ')
+        self.assertEqual([], o.get())
+
+    def test_get_for_api(self):
+        o = types.ListAttribute()
+        o.set_from_cli('foo,bar,baz')
+        self.assertEqual('foo,bar,baz', o.get_for_api())
+
+
+class TestLowercaseStringAttribute(unittest.TestCase):
+    def test_get_for_api(self):
+        o = types.LowercaseStringAttribute('FOO')
+        self.assertEqual('foo', o.get_for_api())
diff --git a/gitlab/types.py b/gitlab/types.py
@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2018 Gauvain Pocentek <gauvain@pocentek.net>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+class GitlabAttribute(object):
+    def __init__(self, value=None):
+        self._value = value
+
+    def get(self):
+        return self._value
+
+    def set_from_cli(self, cli_value):
+        self._value = cli_value
+
+    def get_for_api(self):
+        return self._value
+
+
+class ListAttribute(GitlabAttribute):
+    def set_from_cli(self, cli_value):
+        if not cli_value.strip():
+            self._value = []
+        else:
+            self._value = [item.strip() for item in cli_value.split(',')]
+
+    def get_for_api(self):
+        return ",".join(self._value)
+
+
+class LowercaseStringAttribute(GitlabAttribute):
+    def get_for_api(self):
+        return str(self._value).lower()
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
@@ -45,6 +45,14 @@ def __init__(self, gl, what, action, args):
         self.mgr_cls._path = self.mgr_cls._path % self.args
         self.mgr = self.mgr_cls(gl)
 
+        types = getattr(self.mgr_cls, '_types', {})
+        if types:
+            for attr_name, type_cls in types.items():
+                if attr_name in self.args.keys():
+                    obj = type_cls()
+                    obj.set_from_cli(self.args[attr_name])
+                    self.args[attr_name] = obj.get()
+
     def __call__(self):
         method = 'do_%s' % self.action
         if hasattr(self, method):
diff --git a/gitlab/v4/objects.py b/gitlab/v4/objects.py
@@ -23,6 +23,7 @@
 from gitlab import cli
 from gitlab.exceptions import *  # noqa
 from gitlab.mixins import *  # noqa
+from gitlab import types
 from gitlab import utils
 
 VISIBILITY_PRIVATE = 'private'
@@ -315,12 +316,7 @@ class UserManager(CRUDMixin, RESTManager):
          'website_url', 'skip_confirmation', 'external', 'organization',
          'location')
     )
-
-    def _sanitize_data(self, data, action):
-        new_data = data.copy()
-        if 'confirm' in data:
-            new_data['confirm'] = str(new_data['confirm']).lower()
-        return new_data
+    _types = {'confirm': types.LowercaseStringAttribute}
 
 
 class CurrentUserEmail(ObjectDeleteMixin, RESTObject):
@@ -528,6 +524,7 @@ class GroupIssueManager(GetFromListMixin, RESTManager):
     _obj_cls = GroupIssue
     _from_parent_attrs = {'group_id': 'id'}
     _list_filters = ('state', 'labels', 'milestone', 'order_by', 'sort')
+    _types = {'labels': types.ListAttribute}
 
 
 class GroupMember(SaveMixin, ObjectDeleteMixin, RESTObject):
@@ -736,6 +733,7 @@ class IssueManager(GetFromListMixin, RESTManager):
     _path = '/issues'
     _obj_cls = Issue
     _list_filters = ('state', 'labels', 'order_by', 'sort')
+    _types = {'labels': types.ListAttribute}
 
 
 class License(RESTObject):
@@ -1346,12 +1344,7 @@ class ProjectIssueManager(CRUDMixin, RESTManager):
     _update_attrs = (tuple(), ('title', 'description', 'assignee_id',
                                'milestone_id', 'labels', 'created_at',
                                'updated_at', 'state_event', 'due_date'))
-
-    def _sanitize_data(self, data, action):
-        new_data = data.copy()
-        if 'labels' in data:
-            new_data['labels'] = ','.join(data['labels'])
-        return new_data
+    _types = {'labels': types.ListAttribute}
 
 
 class ProjectMember(SaveMixin, ObjectDeleteMixin, RESTObject):
@@ -1669,12 +1662,7 @@ class ProjectMergeRequestManager(CRUDMixin, RESTManager):
                                'description', 'state_event', 'labels',
                                'milestone_id'))
     _list_filters = ('iids', 'state', 'order_by', 'sort')
-
-    def _sanitize_data(self, data, action):
-        new_data = data.copy()
-        if 'labels' in data:
-            new_data['labels'] = ','.join(data['labels'])
-        return new_data
+    _types = {'labels': types.ListAttribute}
 
 
 class ProjectMilestone(SaveMixin, ObjectDeleteMixin, RESTObject):
