Made changes requested by Vova

Author: JaviCerveraIngram

connect/resource/automation.py

@@ -43,20 +43,17 @@ def process_request(self, request):
     @function_log
     def approve(self, pk, data):
         # type: (str, dict) -> str
-        url = self.urljoin(self.url, pk, 'approve/')
-        return self.api.post(url=url, data=data if data else {})
+        return self.api.post(path=pk + '/approve/', data=data if data else {})
 
     @function_log
     def inquire(self, pk):
         # type: (str) -> str
-        url = self.urljoin(self.url, pk, 'inquire/')
-        return self.api.post(url=url, data={})
+        return self.api.post(path=pk + '/inquire/', data={})
 
     @function_log
     def fail(self, pk, reason):
         # type: (str, str) -> str
-        url = self.urljoin(self.url, pk, 'fail/')
-        return self.api.post(url=url, data={'reason': reason})
+        return self.api.post(path=pk + '/fail/', data={'reason': reason})
 
     @function_log
     def render_template(self, pk, template_id):
@@ -71,6 +68,6 @@ def update_parameters(self, pk, params):
             list_dict.append(_.__dict__ if isinstance(_, Param) else _)
 
         return self.api.put(
-            url=self.urljoin(self.url, pk),
+            path=pk,
             data=json.dumps({'asset': {'params': list_dict}}),
         )

connect/resource/base.py

@@ -18,15 +18,19 @@
 
 
 class ApiClient(object):
-    def __init__(self, config=None):
-        # type: (Config) -> None
+    def __init__(self, config, base_path):
+        # type: (Config, str) -> None
+
+        # Assign base URL
+        self._base_path = base_path
 
         # Assign passed config or globally configured instance
         self._config = config or Config.get_instance()
 
-        # Assert data
-        if not isinstance(self.config, Config):
-            raise ValueError('A valid Config object is required to create an ApiClient')
+    @property
+    def base_path(self):
+        # type: () -> str
+        return self._base_path
 
     @property
     def config(self):
@@ -43,8 +47,47 @@ def headers(self):
             'Content-Type': 'application/json',
         }
 
+    def get_url(self, path=''):
+        # type: (str) -> str
+        return self._urljoin(self.config.api_url, self.base_path, path)
+
+    @function_log
+    def get(self, path='', params=None, **kwargs):
+        kwargs = self._fix_request_kwargs(kwargs, path, params=params)
+        response = requests.get(**kwargs)
+        return self._check_response(response)
+
+    @function_log
+    def post(self, path='', data=None, json=None, **kwargs):
+        kwargs = self._fix_request_kwargs(kwargs, path, data=data, json=json)
+        response = requests.post(**kwargs)
+        return self._check_response(response)
+
+    @function_log
+    def put(self, path='', data=None, **kwargs):
+        kwargs = self._fix_request_kwargs(kwargs, path, data=data)
+        response = requests.put(**kwargs)
+        return self._check_response(response)
+
+    def _fix_request_kwargs(self, prev_kwargs, path, **kwargs):
+        # type: (Dict[str, Any], str, Dict[str, Any]) -> Dict[str, Any]
+        """ Set correct kwargs for requests """
+        fixed_kwargs = prev_kwargs.copy()
+        fixed_kwargs.update(kwargs)
+        if 'get_url' not in fixed_kwargs:
+            fixed_kwargs['get_url'] = self.get_url(path)
+        if 'headers' not in fixed_kwargs:
+            fixed_kwargs['headers'] = self.headers
+        return fixed_kwargs
+
     @staticmethod
-    def check_response(response):
+    def _urljoin(*args):
+        return functools.reduce(
+            lambda a, b: compat.urljoin(a + ('' if a.endswith('/') else '/'), b),
+            args)
+
+    @staticmethod
+    def _check_response(response):
         # type: (requests.Response) -> str
         if not hasattr(response, 'content'):
             raise AttributeError(
@@ -59,47 +102,30 @@ def check_response(response):
 
         return response.content
 
-    @function_log
-    def get(self, url, params=None, **kwargs):
-        if 'headers' not in kwargs:
-            kwargs['headers'] = self.headers
-        response = requests.get(url, params, **kwargs)
-        return self.check_response(response)
-
-    @function_log
-    def post(self, url, data=None, json=None, **kwargs):
-        if 'headers' not in kwargs:
-            kwargs['headers'] = self.headers
-        response = requests.post(url, data, json, **kwargs)
-        return self.check_response(response)
-
-    @function_log
-    def put(self, url, data=None, **kwargs):
-        if 'headers' not in kwargs:
-            kwargs['headers'] = self.headers
-        response = requests.put(url, data, **kwargs)
-        return self.check_response(response)
-
 
 class BaseResource(object):
     resource = None  # type: str
     limit = 100  # type: int
-    api = None  # type: ApiClient
     schema = BaseSchema()  # type: marshmallow.Schema
 
     def __init__(self, config=None):
-        # Assign passed config or globally configured instance
-        self._config = config or Config.get_instance()
-
-        # Assert data
+        # Set api
         if not self.__class__.resource:
-            raise AttributeError('Resource name not specified in class {}'.format(
-                self.__class__.__name__) + '. Add an attribute `resource` name of the resource')
+            raise AttributeError('Resource name not specified in class {}. '
+                                 .format(self.__class__.__name__) +
+                                 'Add an attribute `resource` name of the resource')
+        self._api = ApiClient(config, self.__class__.resource)
+
+        # Set passed config or globally configured instance
+        self._config = config or Config.get_instance()
         if not isinstance(self.config, Config):
             raise ValueError('A valid Config object must be passed or globally configured '
                              'to create a ' + type(self).__name__)
-        if not BaseResource.api:
-            BaseResource.api = ApiClient(config)
+
+    @property
+    def api(self):
+        # type: () -> ApiClient
+        return self._api
 
     @property
     def config(self):
@@ -111,13 +137,9 @@ def list(self):
         # type: () -> List[Any]
         filters = self.build_filter()
         logger.info('Get list request by filter - {}'.format(filters))
-        response = self.api.get(url=self.url, params=filters)
+        response = self.api.get(params=filters)
         return self._load_schema(response)
 
-    @property
-    def url(self):
-        return self.urljoin(self.config.api_url, self.resource)
-
     def build_filter(self):
         # type: () -> Dict[str, Any]
         res_filter = {}
@@ -127,17 +149,11 @@ def build_filter(self):
 
     def get(self, pk):
         # type: (str) -> Any
-        response = self.api.get(url=self.urljoin(self.url, pk))
+        response = self.api.get(path=pk)
         objects = self._load_schema(response)
         if isinstance(objects, list) and len(objects) > 0:
             return objects[0]
 
-    @staticmethod
-    def urljoin(*args):
-        return functools.reduce(
-            lambda a, b: compat.urljoin(a + ('' if a.endswith('/') else '/'), b),
-            args)
-
     def _load_schema(self, response, many=None):
         # type: (str, bool) -> Union[List[Any], Any]
         objects, error = self.schema.loads(response, many)

connect/resource/template.py

@@ -26,10 +26,7 @@ def render(self, pk, request_id):
         # type: (str, str) -> ActivationTileResponse
         if not all([pk, request_id]):
             raise ValueError('Invalid ids for render template')
-
-        url = self.urljoin(self.url, pk, 'render')
-        response = self.api.get(url, params={'request_id': request_id})
-
+        response = self.api.get(path=pk + '/render', params={'request_id': request_id})
         return ActivationTileResponse(response)
 
     def get(self, pk):

connect/resource/tier_config_automation.py

@@ -62,7 +62,7 @@ def get_tier_config(self, tier_id, product_id):
             'configuration__product__id': product_id,
             'configuration__account__id': tier_id,
         }
-        response = self.api.get(url=self.url, params=params)
+        response = self.api.get(params=params)
         objects = self._load_schema(response)
 
         if isinstance(objects, list) and len(objects) > 0:

connect/resource/usage_automation.py

@@ -66,7 +66,7 @@ def create_usage_file(self, usage_file):
         if not usage_file.description:
             # Could be because description is empty or None, so make sure it is empty
             usage_file.description = ''
-        response = self.api.post(self.url, data=usage_file)
+        response = self.api.post(data=usage_file)
         return self._load_schema(response, many=False)
 
     @staticmethod
@@ -105,7 +105,7 @@ def upload_spreadsheet(self, usage_file, spreadsheet):
             file_contents = tmp.read()
 
         # Setup request
-        url = self.urljoin(self.url, usage_file.id, 'upload/')
+        url = self.api.get_url(usage_file.id + '/upload/')
         headers = self.api.headers
         headers['Accept'] = 'application/json'
         del headers['Content-Type']  # This must NOT be set for multipart post requests
@@ -130,10 +130,15 @@ def upload_usage_records(self, usage_file, usage_records):
         self.upload_spreadsheet(usage_file, book)
 
     def get_usage_template(self, product):
-        # type: (Product) -> str
+        # type: (Product) -> bytes
         location = self._get_usage_template_download_location(product.id)
+        if not location:
+            msg = 'Error obtaining template usage file location'
+            logger.error(msg)
+            raise FileRetrievalError(msg)
+
         contents = self._retrieve_usage_template(location) if location else None
-        if not location or contents is None:
+        if not contents:
             msg = 'Error obtaining template usage file from `{}`'.format(location)
             logger.error(msg)
             raise FileRetrievalError(msg)
@@ -148,25 +153,17 @@ def submit_usage(self, usage_file, usage_records):
     def _get_usage_template_download_location(self, product_id):
         # type: (str) -> str
         try:
-            response = self.api.get(self.urljoin(self.config.api_url,
-                                                 '/usage/products/' + product_id + ''))
+            response = self.api.get(path='/usage/products/' + product_id + '/template/')
             response_dict = json.loads(response)
             return response_dict['template_link']
         except (requests.exceptions.RequestException, KeyError, TypeError, ValueError):
             return ''
 
     @staticmethod
     def _retrieve_usage_template(location):
-        # type: (str) -> Optional[str]
+        # type: (str) -> Optional[bytes]
         try:
-            # Try loading from file
-            with open(location) as file_handle:
-                return file_handle.read()
-        except IOError:
-            # Try loading from URL
-            try:
-                file_handle = requests.get(location)
-                return file_handle.text
-            except requests.exceptions.RequestException:
-                # Fail
-                return None
+            response = requests.get(location)
+            return response.content
+        except requests.exceptions.RequestException:
+            return None

connect/resource/usage_file_automation.py

@@ -39,7 +39,7 @@ def dispatch(self, request):
 
         # Catch action
         except UsageFileAction as usage:
-            self.api.post(self.urljoin(self.url, request.id, usage.code), data=usage.obj)
+            self.api.post(path='{}/{}'.format(request.id, usage.code), data=usage.obj)
             processing_result = usage.code
 
         # Catch skip

tests/test_models.py

@@ -35,14 +35,13 @@ def _get_response2_ok():
 
 def test_resource_url():
     resource = FulfillmentAutomation()
-    assert resource.url == '{}{}'.format(resource.config.api_url, resource.resource)
+    assert resource.api.get_url() == resource.config.api_url + resource.resource + '/'
 
 
 def test_resource_urljoin():
     resource = FulfillmentAutomation()
-    assert resource.urljoin('hello', 'world') == 'hello/world'
-    assert resource.urljoin(resource.url, 'hello', 'world') == '{}{}/hello/world' \
-        .format(resource.config.api_url, resource.resource)
+    assert resource.api.base_path == resource.resource
+    assert resource.api.get_url('hello/world') == '{}{}/hello/world'.format(resource.config.api_url, resource.resource)
 
 
 @patch('requests.get', MagicMock(return_value=_get_response_ok()))

tests/test_usage.py

@@ -18,19 +18,19 @@
 from connect.models.product import Product
 from connect.resource import UsageAutomation
 
-Response = namedtuple('Response', ('ok', 'content', 'text', 'status_code'))
+Response = namedtuple('Response', ('ok', 'content', 'status_code'))
 
 
 def _get_response_ok():
     with open(os.path.join(os.path.dirname(__file__), 'response_usage.json')) as file_handle:
         content = file_handle.read()
-    return Response(ok=True, content=content, text='', status_code=200)
+    return Response(ok=True, content=content, status_code=200)
 
 
 def _get_response_ok2():
     with open(os.path.join(os.path.dirname(__file__), 'response_usage2.json')) as file_handle:
         content = file_handle.read()
-    return Response(ok=True, content=content, text='', status_code=201)
+    return Response(ok=True, content=content, status_code=201)
 
 
 @patch('requests.get', MagicMock(return_value=_get_response_ok()))
@@ -47,24 +47,29 @@ def test_process():
     resource.process()
 
 
-@patch('requests.get', MagicMock(return_value=Response(
-    ok=True, content='{"template_link": "..."}', text='test', status_code=200)))
+@patch('requests.get', MagicMock(side_effect=[
+    Response(ok=True, content='{"template_link": "..."}', status_code=200),
+    Response(ok=True, content='template_contents', status_code=200)]))
 def test_get_usage_template_ok():
-    assert UsageAutomation().get_usage_template(Product(id='PRD-638-321-603')) == 'test'
+    resource = UsageAutomation()
+    assert resource.get_usage_template(Product(id='PRD-638-321-603')) == 'template_contents'
 
 
 @patch('requests.get', MagicMock(return_value=Response(
-    ok=True, content='{}', text='', status_code=200)))
+    ok=True, content='{}', status_code=200)))
 def test_get_usage_template_no_link():
+    resource = UsageAutomation()
     with pytest.raises(FileRetrievalError):
-        UsageAutomation().get_usage_template(Product(id='PRD-638-321-603'))
+        resource.get_usage_template(Product(id='PRD-638-321-603'))
 
 
-@patch('requests.get', MagicMock(return_value=Response(
-    ok=True, content='{"template_link": "..."}', text=None, status_code=200)))
+@patch('requests.get', MagicMock(side_effect=[
+    Response(ok=True, content='{"template_link": "..."}', status_code=200),
+    Response(ok=True, content=None, status_code=200)]))
 def test_get_usage_template_no_file():
+    resource = UsageAutomation()
     with pytest.raises(FileRetrievalError):
-        UsageAutomation().get_usage_template(Product(id='PRD-638-321-603'))
+        resource.get_usage_template(Product(id='PRD-638-321-603'))
 
 
 class UsageAutomationTester(UsageAutomation):