ResourceManager refactoring

Author: maxtepkeev

CHANGELOG.rst

@@ -9,6 +9,15 @@ Changelog
 - ResourceSet's ``filter()`` method became more advanced. It is now possible to filter on all available
   resource attributes, to follow resource relationships and apply lookups to the filters (see
   `docs <https://python-redmine.com/introduction.html#methods>`__ for details)
+- ResourceManager class has been refactored:
+
+  * ``manager_class`` attribute on the ``Resource`` class can now be used to assign a separate
+    ``ResourceManager`` to a resource, that allows outsourcing a resource specific functionality to a
+    separate manager class (see ``WikiPageManager`` as an example)
+  * ``request()`` method has been removed
+  * ``_construct_*_url()``, ``_prepare_*_request()``, ``_process_*_response()`` methods have been added
+    for create, update and delete methods to allow a fine-grained control over these operations
+
 - Ability to upload file-like objects (`Issue #186 <https://github.com/maxtepkeev/python-redmine/issues/
   186>`__) (thanks to `hjpotter92 <https://github.com/hjpotter92>`__)
 - Support for retrieving project's time entry activities (see `docs <https://python-redmine.com/resources/

redminelib/__init__.py

@@ -53,7 +53,17 @@ def __getattr__(self, resource_name):
         if resource_name.startswith('_'):
             raise AttributeError
 
-        return managers.ResourceManager(self, resource_name)
+        resource_name = ''.join(word[0].upper() + word[1:] for word in str(resource_name).split('_'))
+
+        try:
+            resource_class = resources.registry[resource_name]['class']
+        except KeyError:
+            raise exceptions.ResourceError
+
+        if self.ver is not None and LooseVersion(str(self.ver)) < LooseVersion(resource_class.redmine_version):
+            raise exceptions.ResourceVersionMismatchError
+
+        return resource_class.manager_class(self, resource_class)
 
     @contextlib.contextmanager
     def session(self, **options):

redminelib/managers/__init__.py

@@ -0,0 +1,6 @@
+"""
+Defines manager classes.
+"""
+
+from .base import ResourceManager
+from .standard import WikiPageManager

redminelib/managers.py redminelib/managers/base.pyredminelib/managers.py renamed to redminelib/managers/base.py

@@ -1,58 +1,25 @@
 """
-Defines manager classes.
+Defines base Redmine resource manager class and it's infrastructure.
 """
 
-from distutils.version import LooseVersion
-
-from . import utilities, resources, resultsets, exceptions
+from .. import utilities, resultsets, exceptions
 
 
 class ResourceManager(object):
     """
-    Manages Redmine resource defined by the resource_name with the help of redmine object.
+    Manages given Redmine resource class with the help of redmine object.
     """
-    def __init__(self, redmine, resource_name, **params):
+    def __init__(self, redmine, resource_class):
         """
         :param redmine.Redmine redmine: (required). Redmine object.
-        :param string resource_name: (required). Resource name.
-        :param dict params: (optional). Parameters used for resources retrieval.
+        :param resources.BaseResource resource_class: (required). Resource class.
         """
-        resource_name = ''.join(word[0].upper() + word[1:] for word in str(resource_name).split('_'))
-
-        try:
-            resource_class = resources.registry[resource_name]['class']
-        except KeyError:
-            raise exceptions.ResourceError
-
-        if redmine.ver is not None and LooseVersion(str(redmine.ver)) < LooseVersion(resource_class.redmine_version):
-            raise exceptions.ResourceVersionMismatchError
-
         self.url = ''
+        self.params = {}
         self.container = None
-        self.params = params
         self.redmine = redmine
         self.resource_class = resource_class
 
-    def request(self, is_bulk, **params):
-        """
-        Makes request(s) and additionally checks for resource specific stuff.
-
-        :param bool is_bulk: (required). Whether this is a bulk or single request.
-        :param dict params: (optional). Parameters used for resource retrieval.
-        """
-        try:
-            if not is_bulk:
-                return self.redmine.engine.request('get', self.url, params=params)[self.container]
-            else:
-                return self.redmine.engine.bulk_request('get', self.url, self.container, **self.params)
-        except exceptions.ResourceNotFoundError as e:
-            # This is the only place we're checking for ResourceRequirementsError
-            # because for some POST/PUT/DELETE requests Redmine may also return 404
-            # status code instead of 405 which can lead us to improper decisions
-            if self.resource_class.requirements:
-                raise exceptions.ResourceRequirementsError(self.resource_class.requirements)
-            raise e
-
     def to_resource(self, resource):
         """
         Converts resource data to Resource object.
@@ -83,7 +50,9 @@ def new_manager(self, resource_name, **params):
         :param string resource_name: (required). Resource name.
         :param dict params: (optional). Parameters used for resources retrieval.
         """
-        return ResourceManager(self.redmine, resource_name, **params)
+        manager = getattr(self.redmine, resource_name)
+        manager.params = params
+        return manager
 
     def get(self, resource_id, **params):
         """
@@ -109,7 +78,13 @@ def get(self, resource_id, **params):
 
         self.params = self.resource_class.bulk_decode(params, self)
         self.container = self.resource_class.container_one
-        return self.to_resource(self.request(False, **self.params))
+
+        try:
+            return self.to_resource(self.redmine.engine.request('get', self.url, params=self.params)[self.container])
+        except exceptions.ResourceNotFoundError as e:
+            if self.resource_class.requirements:
+                raise exceptions.ResourceRequirementsError(self.resource_class.requirements)
+            raise e
 
     def all(self, **params):
         """
@@ -146,6 +121,22 @@ def filter(self, **filters):
         self.params = self.resource_class.bulk_decode(filters, self)
         return resultsets.ResourceSet(self)
 
+    def _construct_create_url(self, path):
+        """
+        Constructs URL for create method.
+
+        :param string path: absolute URL path.
+        """
+        return self.redmine.url + path
+
+    def _prepare_create_request(self, request):
+        """
+        Makes the necessary preparations for create request data.
+
+        :param dict request: Request data.
+        """
+        return {self.container: self.resource_class.bulk_decode(request, self)}
+
     def create(self, **fields):
         """
         Creates a new resource in Redmine and returns created Resource object on success.
@@ -161,30 +152,50 @@ def create(self, **fields):
         formatter = utilities.MemorizeFormatter()
 
         try:
-            url = self.redmine.url + formatter.format(self.resource_class.query_create, **fields)
-        except KeyError as exception:
-            raise exceptions.ValidationError('{0} field is required'.format(exception))
-
-        self.container = self.resource_class.container_create
-        data = {self.resource_class.container_create: self.resource_class.bulk_decode(formatter.unused_kwargs, self)}
-        response = self.redmine.engine.request(self.resource_class.http_method_create, url, data=data)
-
-        try:
-            resource = self.to_resource(response[self.container])
-        except TypeError:
-            raise exceptions.ValidationError('Resource already exists')  # fix for repeated PUT requests (issue #182)
+            url = self._construct_create_url(formatter.format(self.resource_class.query_create, **fields))
+        except KeyError as e:
+            raise exceptions.ValidationError('{0} field is required'.format(e))
 
         self.params = formatter.used_kwargs
+        self.container = self.resource_class.container_create
+        request = self._prepare_create_request(formatter.unused_kwargs)
+        response = self.redmine.engine.request(self.resource_class.http_method_create, url, data=request)
+        resource = self._process_create_response(request, response)
         self.url = self.redmine.url + self.resource_class.query_one.format(resource.internal_id, **fields)
         return resource
 
+    def _process_create_response(self, request, response):
+        """
+        Processes create response and constructs resource object.
+
+        :param dict request: Original request data.
+        :param any response: Response received from Redmine for this request data.
+        """
+        return self.to_resource(response[self.container])
+
+    def _construct_update_url(self, path):
+        """
+        Constructs URL for update method.
+
+        :param string path: absolute URL path.
+        """
+        return self.redmine.url + path
+
+    def _prepare_update_request(self, request):
+        """
+        Makes the necessary preparations for update request data.
+
+        :param dict request: Request data.
+        """
+        return {self.resource_class.container_update: self.resource_class.bulk_decode(request, self)}
+
     def update(self, resource_id, **fields):
         """
         Updates a Resource object by resource id.
 
         :param resource_id: (required). Resource id.
         :type resource_id: int or string
-        :param dict fields: (optional). Fields which will be updated for the resource.
+        :param dict fields: (optional). Fields that will be updated for the resource.
         """
         if self.resource_class.query_update is None or self.resource_class.container_update is None:
             raise exceptions.ResourceBadMethodError
@@ -196,18 +207,44 @@ def update(self, resource_id, **fields):
 
         try:
             query_update = formatter.format(self.resource_class.query_update, resource_id, **fields)
-        except KeyError as exception:
-            param = exception.args[0]
+        except KeyError as e:
+            param = e.args[0]
 
             if param in self.params:
                 fields[param] = self.params[param]
                 query_update = formatter.format(self.resource_class.query_update, resource_id, **fields)
             else:
-                raise exceptions.ValidationError('{0} argument is required'.format(exception))
+                raise exceptions.ValidationError('{0} argument is required'.format(e))
 
-        url = self.redmine.url + query_update
-        data = {self.resource_class.container_update: self.resource_class.bulk_decode(formatter.unused_kwargs, self)}
-        return self.redmine.engine.request(self.resource_class.http_method_update, url, data=data)
+        url = self._construct_update_url(query_update)
+        request = self._prepare_update_request(formatter.unused_kwargs)
+        response = self.redmine.engine.request(self.resource_class.http_method_update, url, data=request)
+        return self._process_update_response(request, response)
+
+    def _process_update_response(self, request, response):
+        """
+        Processes update response.
+
+        :param dict request: Original request data.
+        :param any response: Response received from Redmine for this request data.
+        """
+        return response
+
+    def _construct_delete_url(self, path):
+        """
+        Constructs URL for delete method.
+
+        :param string path: absolute URL path.
+        """
+        return self.redmine.url + path
+
+    def _prepare_delete_request(self, request):
+        """
+        Makes the necessary preparations for delete request data.
+
+        :param dict request: Request data.
+        """
+        return self.resource_class.bulk_decode(request, self)
 
     def delete(self, resource_id, **params):
         """
@@ -221,12 +258,22 @@ def delete(self, resource_id, **params):
             raise exceptions.ResourceBadMethodError
 
         try:
-            url = self.redmine.url + self.resource_class.query_delete.format(resource_id, **params)
-        except KeyError as exception:
-            raise exceptions.ValidationError('{0} argument is required'.format(exception))
+            url = self._construct_delete_url(self.resource_class.query_delete.format(resource_id, **params))
+        except KeyError as e:
+            raise exceptions.ValidationError('{0} argument is required'.format(e))
 
-        return self.redmine.engine.request(
-            self.resource_class.http_method_delete, url, params=self.resource_class.bulk_decode(params, self))
+        request = self._prepare_delete_request(params)
+        response = self.redmine.engine.request(self.resource_class.http_method_delete, url, params=request)
+        return self._process_delete_response(request, response)
+
+    def _process_delete_response(self, request, response):
+        """
+        Processes delete response.
+
+        :param dict request: Original request data.
+        :param any response: Response received from Redmine for this request data.
+        """
+        return response
 
     def search(self, query, **options):
         """
@@ -246,5 +293,5 @@ def __repr__(self):
         """
         Official representation of a ResourceManager object.
         """
-        return '<{0}.{1} object for {2} resource>'.format(
-            self.__class__.__module__, self.__class__.__name__, self.resource_class.__name__)
+        return '<redminelib.managers.{0} object for {1} resource>'.format(
+            self.__class__.__name__, self.resource_class.__name__)

redminelib/managers/standard.py

@@ -0,0 +1,14 @@
+"""
+Defines standard Redmine resources managers.
+"""
+
+from . import ResourceManager
+from .. import exceptions
+
+
+class WikiPageManager(ResourceManager):
+    def _process_create_response(self, request, response):
+        if response is True:
+            raise exceptions.ValidationError('Resource already exists')  # issue #182
+
+        return super(WikiPageManager, self)._process_create_response(request, response)

redminelib/resources/base.py

@@ -6,7 +6,7 @@
 
 from datetime import date, datetime
 
-from .. import utilities, exceptions
+from .. import managers, utilities, exceptions
 
 
 registry = {}
@@ -100,6 +100,7 @@ class BaseResource(utilities.with_metaclass(Registrar)):
     http_method_create = 'post'
     http_method_update = 'put'
     http_method_delete = 'delete'
+    manager_class = managers.ResourceManager
 
     _repr = [['id', 'name']]
     _includes = []

redminelib/resources/standard.py

@@ -7,7 +7,7 @@
 from distutils.version import LooseVersion
 
 from . import BaseResource
-from .. import exceptions
+from .. import managers, exceptions
 
 
 class Project(BaseResource):
@@ -246,6 +246,7 @@ class WikiPage(BaseResource):
     query_delete = '/projects/{project_id}/wiki/{0}.json'
     search_hints = ['wiki-page']
     http_method_create = 'put'
+    manager_class = managers.WikiPageManager
 
     _repr = [['title']]
     _includes = ['attachments']

redminelib/resultsets.py

@@ -105,7 +105,15 @@ def __iter__(self):
         if self._resources is None:
             self.manager.params.setdefault('limit', self.limit)
             self.manager.params.setdefault('offset', self.offset)
-            self._resources, self._total_count = self.manager.request(True)
+
+            try:
+                self._resources, self._total_count = self.manager.redmine.engine.bulk_request(
+                    'get', self.manager.url, self.manager.container, **self.manager.params)
+            except exceptions.ResourceNotFoundError as e:
+                if self.manager.resource_class.requirements:
+                    raise exceptions.ResourceRequirementsError(self.manager.resource_class.requirements)
+                raise e
+
             resources = self._resources
         # Otherwise ResourceSet object should handle slicing by itself
         elif self._is_sliced:

tests/test_managers.py

@@ -229,11 +229,13 @@ def test_create_validation_exception_via_put(self):
     def test_reraises_not_found_exception(self):
         self.response.status_code = 404
         self.assertRaises(exceptions.ResourceNotFoundError, lambda: self.redmine.project.get('non-existent-project'))
+        self.assertRaises(exceptions.ResourceNotFoundError, lambda: list(self.redmine.project.all()))
 
     def test_resource_requirements_exception(self):
         FooResource.requirements = ('foo plugin', ('bar plugin', '1.2.3'),)
         self.response.status_code = 404
         self.assertRaises(exceptions.ResourceRequirementsError, lambda: self.redmine.foo_resource.get(1))
+        self.assertRaises(exceptions.ResourceRequirementsError, lambda: list(self.redmine.foo_resource.all()))
 
     def test_search(self):
         self.response.json.return_value = {'total_count': 1, 'offset': 0, 'limit': 0, 'results': [