Merge pull request #16 from JaviCerveraIngram/master

Reducing dependencies from global objects: Config instance is now passed on creation of resources.

Author: vgrebenschikov

README.md

@@ -43,8 +43,6 @@ from connect.logger import logger
 from connect.models import ActivationTemplateResponse, ActivationTileResponse
 from connect.models.exception import FulfillmentFail, FulfillmentInquire, Skip
 
-Config(file='config.json')
-
 # set logger level / default level ERROR
 logger.setLevel("DEBUG")
 
@@ -68,12 +66,12 @@ class ExampleRequestProcessor(FulfillmentAutomation):
             return ActivationTileResponse(tile='\n  # Welcome to Fallball!\n\nYes, '
                                                'you decided to have an account in our amazing service!')
             # or
-            # return TemplateResource().render(pk='TEMPLATE_ID', request_id=request.id)
+            # return TemplateResource(self.config).render(pk='TEMPLATE_ID', request_id=request.id)
 
             # aprrove by Template
             return ActivationTemplateResponse(template_id="TL-497-535-242")
             # or
-            # return TemplateResource().get(pk='TEMPLATE_ID')
+            # return TemplateResource(self.config).get(pk='TEMPLATE_ID')
 
         elif request.type == 'change':
             # fail
@@ -84,7 +82,7 @@ class ExampleRequestProcessor(FulfillmentAutomation):
 
 
 if __name__ == '__main__':
-    request = ExampleRequestProcessor()
+    request = ExampleRequestProcessor(Config(file='config.json'))
     request.process()
 ```
 

connect/config.py

@@ -10,97 +10,72 @@
 
 
 class Config(object):
-    api_url = None
-    api_key = None
-    products = []
+    instance = None  # Global instance
 
     def __init__(
             self,
             api_url=None,
             api_key=None,
             products=None,
-            file=None,
-            **kwargs
+            file=None
     ):
         """
         initialization config for public api
         :param api_url: Public api url
         :param api_key: Service user ApiKey
         :param products (optional): Id products
         :param file: Config file path
-        :param kwargs:
         """
-        if not all([Config.api_key, Config.api_url]):
-            self.load(api_url, api_key, products, file, **kwargs)
-
-    def load(self, api_url, api_key, products, file, **kwargs):
-        if not any([api_key, api_url, file]):
-            return
-
-        if file:
-            return self.load_from_file(file)
-
-        self.check_credentials(api_url, api_key, products)
-        self._set_attr(api_url, api_key, products)
-
-    @staticmethod
-    def _set_attr(api_url, api_key, products=None):
-        Config.api_key = api_key
-
-        # URL must end with a character
-        Config.api_url = api_url
-        if products:
-            Config.products = [products] if isinstance(
-                products, str) else products
-
-    @staticmethod
-    def check_credentials(api_url, api_key, products):
-        """
-        :param api_url:
-        :param api_key:
-        :param products:
-        :return: True or raise ValueError
-        """
-        if not all([api_key, api_url]):
-            raise ValueError('Please provide your credentials.'
-                             'Not set value for `api_key` or `api_url`')
-
+        # Check arguments
+        if not file and not any([api_key, api_url]):
+            raise ValueError('Filename or api_key and api_url are expected'
+                             'in Config initialization')
         if products and not isinstance(products, (str, list)):
             raise TypeError('Products can be string or string list. Found type '
                             + type(products).__name__)
 
-        return
-
-    def load_from_file(self, file):
-        """
-        Format file (json): {
-            "api_key":
-            "api_url":
-            "products" (optional):
-        }
-        :param file (str): Path to the file
-        :return: Init configuration parameters.
-                Set Config.api_url/.api_key/.products
-        """
-        if not os.path.exists(file):
-            raise IOError('Not file `{}` on directory'.format(file))
-
-        with open(file) as config_file:
-            configs = config_file.read()
-
-        try:
-            configs = json.loads(configs)
-        except Exception as ex:
-            raise TypeError('Invalid config file `{}`\n'
-                            'ERROR: {}'.format(file, str(ex)))
-
-        (api_url, api_key, products) = (configs.get('apiEndpoint', ''),
-                                        configs.get('apiKey', ''),
-                                        configs.get('products', ''))
-
-        products = products.encode('utf-8') if not isinstance(products, (str, list)) else products
-        api_url = api_url.encode('utf-8') if not isinstance(api_url, str) else api_url
-        api_key = api_key.encode('utf-8') if not isinstance(api_key, str) else api_key
-
-        self.check_credentials(api_url, api_key, products)
-        self._set_attr(api_url, api_key, products)
+        # Load config from file name
+        if file:
+            if not os.path.exists(file):
+                raise IOError('Not file `{}` on directory'.format(file))
+
+            with open(file) as config_file:
+                configs = config_file.read()
+
+            try:
+                configs = json.loads(configs)
+            except Exception as ex:
+                raise TypeError('Invalid config file `{}`\n'
+                                'ERROR: {}'.format(file, str(ex)))
+
+            (api_url, api_key, products) = (configs.get('apiEndpoint', ''),
+                                            configs.get('apiKey', ''),
+                                            configs.get('products', ''))
+            api_url = api_url.encode('utf-8') if not isinstance(api_url, str) else api_url
+            api_key = api_key.encode('utf-8') if not isinstance(api_key, str) else api_key
+            products = products.encode('utf-8') \
+                if not isinstance(products, (str, list)) \
+                else products
+
+        # Initialize
+        self._api_key = api_key
+        self._api_url = api_url
+        self._products = [products] \
+            if isinstance(products, str) \
+            else products or []
+
+        # Store first created instance
+        if not Config.instance:
+            Config.instance = self
+
+    @property
+    def api_url(self):
+        return self._api_url
+
+    @property
+    def api_key(self):
+        return self._api_key
+
+    @property
+    def products(self):
+        return self._products

connect/resource/base.py

@@ -13,17 +13,21 @@
 from connect.models.exception import ServerErrorException
 from .utils import joinurl
 
-config = Config()
-
 
 class ApiClient(object):
 
+    def __init__(self, config=None):
+        # Assign passed config or globally configured instance
+        self.config = config or Config.instance
+
+        # Assert data
+        if not isinstance(self.config, Config):
+            raise ValueError('A valid Config object is required to create an ApiClient')
+
     @property
     def headers(self):
-        config.check_credentials(
-            config.api_url, config.api_key, config.products)
         return {
-            "Authorization": config.api_key,
+            "Authorization": self.config.api_key,
             "Content-Type": "application/json",
         }
 
@@ -64,14 +68,22 @@ def put(self, url, data=None, **kwargs):
 class BaseResource(object):
     resource = None
     limit = 100
-    api = ApiClient()
+    api = None
     schema = BaseSchema()
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, config=None, *args, **kwargs):
+        # Assign passed config or globally configured instance
+        self.config = config or Config.instance
 
-        if self.__class__.resource is None:
+        # Assert data
+        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')
+        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)
 
     def build_filter(self):
         res_filter = {}
@@ -82,7 +94,7 @@ def build_filter(self):
 
     @property
     def _list_url(self):
-        return joinurl(config.api_url, self.__class__.resource)
+        return joinurl(self.config.api_url, self.__class__.resource)
 
     def _obj_url(self, pk):
         return joinurl(self._list_url, pk)

connect/resource/fulfillment.py

@@ -7,7 +7,6 @@
 
 import json
 
-from connect.config import Config
 from connect.logger import function_log
 from connect.models import FulfillmentSchema, Param
 from .base import BaseResource
@@ -22,8 +21,8 @@ class FulfillmentResource(BaseResource):
 
     def build_filter(self):
         filters = super(FulfillmentResource, self).build_filter()
-        if Config.products:
-            filters['product_id'] = Config.products
+        if self.config.products:
+            filters['product_id'] = self.config.products
 
         filters['status'] = 'pending'
         return filters
@@ -44,7 +43,7 @@ def fail(self, pk, reason):
 
     @function_log
     def render_template(self, pk, template_id):
-        return TemplateResource().render(template_id, pk)
+        return TemplateResource(self.config).render(template_id, pk)
 
     @function_log
     def update_parameters(self, pk, params):

example/example.py

@@ -11,8 +11,6 @@
 from connect.models import ActivationTemplateResponse, ActivationTileResponse
 from connect.models.exception import FulfillmentFail, FulfillmentInquire, Skip
 
-Config(file='config.json')
-
 # set logger level / default level ERROR
 logger.setLevel("DEBUG")
 
@@ -36,12 +34,12 @@ def process_request(self, request):
             return ActivationTileResponse(tile='\n  # Welcome to Fallball!\n\nYes, you decided '
                                                'to have an account in our amazing service!')
             # or
-            # return TemplateResource().render(pk='TEMPLATE_ID', request_id=request.id)
+            # return TemplateResource(self.config).render(pk='TEMPLATE_ID', request_id=request.id)
 
             # aprrove by Template
             return ActivationTemplateResponse(template_id="TL-497-535-242")
             # or
-            # return TemplateResource().get(pk='TEMPLATE_ID')
+            # return TemplateResource(self.config).get(pk='TEMPLATE_ID')
 
         elif request.type == 'change':
             # fail
@@ -52,5 +50,5 @@ def process_request(self, request):
 
 
 if __name__ == '__main__':
-    request = ExampleRequestProcessor()
+    request = ExampleRequestProcessor(Config(file='config.json'))
     request.process()

tests/test_config.py

@@ -28,38 +28,65 @@ def teardown_module(module):
     os.chdir(module.prev_dir)
 
 
-def teardown_function():
-    Config.api_url, Config.api_key, Config.products = None, None, None
-
-
 def test_init_config_with_non_existing_file():
     with pytest.raises(IOError):
         Config(file='non_existing_config.json')
 
 
 def test_init_config_with_file():
-    config = Config(file='config.json')
-    _assert_config(config)
+    _assert_config(Config(file='config.json'))
 
 
-def test_set_config():
-    config = Config(
+def test_init_config_with_arguments():
+    _assert_config(Config(
         api_key=conf_dict.get('apiKey'),
         api_url=conf_dict.get('apiEndpoint'),
         products=conf_dict.get('products'),
-    )
-    _assert_config(config)
-
-
-def _assert_config(config=None):
-    assert Config.api_key == conf_dict.get('apiKey')
-    assert Config.api_url == conf_dict.get('apiEndpoint')
-    assert isinstance(Config.products, list)
-    assert len(Config.products) == 1
-    assert Config.products == [conf_dict.get('products')]
-    if config:
-        assert config.api_key == conf_dict.get('apiKey')
-        assert config.api_url == conf_dict.get('apiEndpoint')
-        assert isinstance(config.products, list)
-        assert len(config.products) == 1
-        assert config.products == [conf_dict.get('products')]
+    ))
+
+
+def test_init_config_with_invalid_arguments():
+    with pytest.raises(ValueError):
+        Config(
+            api_key='',
+            api_url='',
+            products='',
+        )
+
+
+def test_config_immutable_properties():
+    config = Config(file='config.json')
+    with pytest.raises(AttributeError):
+        config.api_key = conf_dict.get('apiKey')
+        config.api_url = conf_dict.get('apiEndpoint')
+        config.products = [conf_dict.get('products')]
+
+
+def test_global_config():
+    Config.instance = None  # Reset global config
+    Config(file='config.json')
+    _assert_global_config()
+
+
+def test_global_config_immutable_properties():
+    Config(file='config.json')
+    with pytest.raises(AttributeError):
+        Config.instance.api_key = conf_dict.get('apiKey')
+        Config.instance.api_url = conf_dict.get('apiEndpoint')
+        Config.instance.products = [conf_dict.get('products')]
+
+
+def _assert_config(config):
+    assert config.api_key == conf_dict.get('apiKey')
+    assert config.api_url == conf_dict.get('apiEndpoint')
+    assert isinstance(config.products, list)
+    assert len(config.products) == 1
+    assert config.products[0] == conf_dict.get('products')
+
+
+def _assert_global_config():
+    assert Config.instance.api_key == conf_dict.get('apiKey')
+    assert Config.instance.api_url == conf_dict.get('apiEndpoint')
+    assert isinstance(Config.instance.products, list)
+    assert len(Config.instance.products) == 1
+    assert Config.instance.products[0] == conf_dict.get('products')