improvements (#62)

bartes · web-flow · commit 9a5b91200edb · 2020-03-26T16:41:59.000+01:00
* moved request/response/session to apis namespace

* added config validation before sending request

* updated version

* renamed session_sharer to session

* updated travis.yml

* added p3.8 to travis
diff --git a/.python-version b/.python-version
@@ -1 +1 @@
-3.6.6
+3.7.7
diff --git a/.travis.yml b/.travis.yml
@@ -1,15 +1,16 @@
 dist: xenial
 language: python
+os: linux
 
 python:
   - "3.4"
   - "3.5"
   - "3.6"
+  - "3.7"
 
-matrix:
+jobs:
   include:
-    - python: "3.7"
-      sudo: true
+    - python: "3.8"
       env:
       - REQUESTS="requests" # latest
 
diff --git a/HISTORY.md b/HISTORY.md
@@ -1,8 +1,7 @@
 ## master
 
 - [#61](https://github.com/castle/castle-python/pull/61) improve headers and ip extractions, improve ip_headers config, add trusted proxies config, added more events to events list
-
-https://github.com/castle/castle-python/pull/61
+- [#62](https://github.com/castle/castle-python/pull/62) move request,response, session to apis namespace, add config check before doing request
 
 ## 3.0.0 (2020-02-13)
 
diff --git a/castle/api.py b/castle/api.py
@@ -1,13 +1,17 @@
-from castle.request import Request
-from castle.response import Response
+from castle.apis.request import ApisRequest
+from castle.apis.response import ApisResponse
+from castle.configuration import configuration
+from castle.exceptions import ConfigurationError
 
 
 class Api(object):
     def __init__(self):
-        self.req = Request({'Content-Type': 'application/json'})
+        self.req = ApisRequest({'Content-Type': 'application/json'})
 
     def request(self, command):
+        if not configuration.isValid():
+            raise ConfigurationError
         return self.req.build_query(command.method, command.path, command.data)
 
     def call(self, command):
-        return Response(self.request(command)).call()
+        return ApisResponse(self.request(command)).call()
diff --git a/castle/apis/__init__.py b/castle/apis/__init__.py
diff --git a/castle/apis/request.py b/castle/apis/request.py
@@ -1,22 +1,22 @@
 import json
 from castle.configuration import configuration
-from castle.session import SessionSharer
+from castle.apis.session import ApisSession
 
 
-class Request(object):
+class ApisRequest(object):
     def __init__(self, headers=None):
         self.headers = headers or dict()
-        self.base_url = Request.build_base_url()
-        self.sharer = SessionSharer()
+        self.base_url = ApisRequest.build_base_url()
+        self.session = ApisSession()
 
     def build_query(self, method, path, params):
-        return self.sharer.session.request(
+        return self.session.get().request(
             method,
             self.build_url(path),
             auth=('', configuration.api_secret),
             timeout=configuration.request_timeout / 1000.0,
             headers=self.headers,
-            verify=Request.verify(),
+            verify=ApisRequest.verify(),
             data=None if params is None else json.dumps(params)
         )
 
diff --git a/castle/apis/response.py b/castle/apis/response.py
@@ -12,7 +12,7 @@
 }
 
 
-class Response(object):
+class ApisResponse(object):
     def __init__(self, response):
         self.response = response
 
diff --git a/castle/apis/session.py b/castle/apis/session.py
@@ -0,0 +1,19 @@
+import requests
+
+
+class ApisSession(object):
+    class __ApisSession:
+        def __init__(self):
+            self.session = requests.Session()
+
+        def get(self):
+            return self.session
+    instance = None
+
+    def __new__(cls):
+        if not ApisSession.instance:
+            ApisSession.instance = ApisSession.__ApisSession()
+        return ApisSession.instance
+
+    def get(self):
+        return self.instance.get()
diff --git a/castle/configuration.py b/castle/configuration.py
@@ -51,6 +51,9 @@ def __init__(self):
         self.ip_headers = []
         self.trusted_proxies = []
 
+    def isValid(self):
+        return self.host and self.port and self.api_secret
+
     @property
     def api_secret(self):
         return self.__api_secret
diff --git a/castle/session.py b/castle/session.py
diff --git a/castle/test/__init__.py b/castle/test/__init__.py
@@ -5,6 +5,9 @@
 
 
 TEST_MODULES = [
+    'castle.test.apis.request_test',
+    'castle.test.apis.response_test',
+    'castle.test.apis.session_test',
     'castle.test.api_test',
     'castle.test.client_test',
     'castle.test.configuration_test',
@@ -22,11 +25,8 @@
     'castle.test.extractors.ip_test',
     'castle.test.failover_response_test',
     'castle.test.headers_formatter_test',
-    'castle.test.request_test',
-    'castle.test.response_test',
     'castle.test.review_test',
     'castle.test.secure_mode_test',
-    'castle.test.session_test',
     'castle.test.validators.not_supported_test',
     'castle.test.validators.present_test',
     'castle.test.utils_test'
diff --git a/castle/test/api_test.py b/castle/test/api_test.py
@@ -4,7 +4,9 @@
 from castle.test import unittest
 from castle.api import Api
 from castle.command import Command
-from castle.request import Request
+from castle.apis.request import ApisRequest
+from castle.configuration import configuration
+from castle.exceptions import ConfigurationError
 
 
 def command():
@@ -16,8 +18,14 @@ def response_text():
 
 
 class ApiTestCase(unittest.TestCase):
+    def setUp(self):
+        configuration.api_secret = 'test'
+
+    def tearDown(self):
+        configuration.api_secret = None
+
     def test_init(self):
-        self.assertIsInstance(Api().req, Request)
+        self.assertIsInstance(Api().req, ApisRequest)
 
     @responses.activate
     def test_request(self):
@@ -38,3 +46,9 @@ def test_call(self):
             status=200
         )
         self.assertEqual(Api().call(command()), response_text())
+
+    @responses.activate
+    def test_no_api_secret(self):
+        configuration.api_secret = ''
+        with self.assertRaises(ConfigurationError):
+            Api().call(command())
diff --git a/castle/test/apis/__init__.py b/castle/test/apis/__init__.py
diff --git a/castle/test/apis/request_test.py b/castle/test/apis/request_test.py
@@ -2,7 +2,7 @@
 from requests import Response
 import responses
 from castle.test import unittest
-from castle.request import Request
+from castle.apis.request import ApisRequest
 from castle.configuration import configuration
 
 try:
@@ -28,13 +28,13 @@ def do_POST(self):
     return httpd_thread
 
 
-class RequestTestCase(unittest.TestCase):
+class ApisRequestTestCase(unittest.TestCase):
     def test_init_headers(self):
         headers = {'X-Castle-Client-Id': '1234'}
-        self.assertEqual(Request(headers).headers, headers)
+        self.assertEqual(ApisRequest(headers).headers, headers)
 
     def test_init_base_url(self):
-        self.assertEqual(Request().base_url, 'https://api.castle.io/v1')
+        self.assertEqual(ApisRequest().base_url, 'https://api.castle.io/v1')
 
     @responses.activate
     def test_build_query(self):
@@ -48,7 +48,7 @@ def test_build_query(self):
             json=response_text,
             status=200
         )
-        res = Request().build_query('post', 'authenticate', data)
+        res = ApisRequest().build_query('post', 'authenticate', data)
         self.assertIsInstance(res, Response)
         self.assertEqual(res.status_code, 200)
         self.assertEqual(res.json(), response_text)
@@ -59,7 +59,7 @@ def test_connection_pooled(self):
         configuration.host = 'localhost'
         configuration.port = 65521
         run_server()
-        request = Request()
+        request = ApisRequest()
         data = {'event': '$login.authenticate', 'user_id': '12345'}
         response = request.build_query('post', 'authenticate', data)
         num_pools = len(response.connection.poolmanager.pools.keys())
@@ -69,14 +69,14 @@ def test_connection_pooled(self):
 
     def test_build_url(self):
         self.assertEqual(
-            Request().build_url('authenticate'),
+            ApisRequest().build_url('authenticate'),
             'https://api.castle.io/v1/authenticate'
         )
 
     def test_verify_true(self):
-        self.assertEqual(Request().verify(), True)
+        self.assertEqual(ApisRequest().verify(), True)
 
     def test_verify_false(self):
         configuration.port = 3001
-        self.assertEqual(Request().verify(), False)
+        self.assertEqual(ApisRequest().verify(), False)
         configuration.port = 443
diff --git a/castle/test/apis/response_test.py b/castle/test/apis/response_test.py
@@ -2,7 +2,7 @@
 import requests
 
 from castle.test import unittest
-from castle.response import Response
+from castle.apis.response import ApisResponse
 from castle.exceptions import BadRequestError, UnauthorizedError, ForbiddenError, NotFoundError, \
     UserUnauthorizedError, InvalidParametersError, InternalServerError
 
@@ -14,49 +14,49 @@ def response(status_code=200, body=None):
     return resp
 
 
-class ResponseTestCase(unittest.TestCase):
+class ApisResponseTestCase(unittest.TestCase):
     def test_response_none(self):
-        self.assertEqual(Response(response()).call(), {})
+        self.assertEqual(ApisResponse(response()).call(), {})
 
     def test_response_empty(self):
-        self.assertEqual(Response(response(body=b'')).call(), {})
+        self.assertEqual(ApisResponse(response(body=b'')).call(), {})
 
     def test_response_authenticate(self):
         self.assertEqual(
-            Response(
+            ApisResponse(
                 response(body=b'{"action":"allow","user_id":"12345"}')).call(),
             {"action": "allow", "user_id": "12345"}
         )
 
     def test_verify_200_299(self):
         for status_code in range(200, 299):
             self.assertEqual(
-                Response(response(status_code=status_code)).verify(), None)
+                ApisResponse(response(status_code=status_code)).verify(), None)
 
     def test_verify_400(self):
         with self.assertRaises(BadRequestError):
-            Response(response(status_code=400)).verify()
+            ApisResponse(response(status_code=400)).verify()
 
     def test_verify_401(self):
         with self.assertRaises(UnauthorizedError):
-            Response(response(status_code=401)).verify()
+            ApisResponse(response(status_code=401)).verify()
 
     def test_verify_403(self):
         with self.assertRaises(ForbiddenError):
-            Response(response(status_code=403)).verify()
+            ApisResponse(response(status_code=403)).verify()
 
     def test_verify_404(self):
         with self.assertRaises(NotFoundError):
-            Response(response(status_code=404)).verify()
+            ApisResponse(response(status_code=404)).verify()
 
     def test_verify_419(self):
         with self.assertRaises(UserUnauthorizedError):
-            Response(response(status_code=419)).verify()
+            ApisResponse(response(status_code=419)).verify()
 
     def test_verify_422(self):
         with self.assertRaises(InvalidParametersError):
-            Response(response(status_code=422)).verify()
+            ApisResponse(response(status_code=422)).verify()
 
     def test_verify_500(self):
         with self.assertRaises(InternalServerError):
-            Response(response(status_code=500)).verify()
+            ApisResponse(response(status_code=500)).verify()
diff --git a/castle/test/apis/session_test.py b/castle/test/apis/session_test.py
@@ -13,9 +13,9 @@ def request():
     return req
 
 
-class SessionTestCase(unittest.TestCase):
+class ApisSessionTestCase(unittest.TestCase):
     def test_init(self):
         client = Client.from_request(request(), {})
         client2 = Client.from_request(request(), {})
         self.assertNotEqual(client.api.request, client2.api.request)
-        self.assertEqual(client.api.req.sharer, client2.api.req.sharer)
+        self.assertEqual(client.api.req.session.get(), client2.api.req.session.get())
diff --git a/castle/test/client_test.py b/castle/test/client_test.py
@@ -26,6 +26,10 @@ def setUp(self):
         self.mock_timestamp = timestamp_patcher.start()
         self.mock_timestamp.return_value = '2018-01-02T03:04:05.678'
         self.addCleanup(timestamp_patcher.stop)
+        configuration.api_secret = 'test'
+
+    def tearDown(self):
+        configuration.api_secret = None
 
     def test_init(self):
         context = {
diff --git a/castle/test/review_test.py b/castle/test/review_test.py
@@ -3,9 +3,16 @@
 
 from castle.test import unittest
 from castle.review import Review
+from castle.configuration import configuration
 
 
 class ReviewTestCase(unittest.TestCase):
+    def setUp(self):
+        configuration.api_secret = 'test'
+
+    def tearDown(self):
+        configuration.api_secret = None
+
     @responses.activate
     def test_retrieve(self):
         # pylint: disable=line-too-long

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`}`
`13`	`13`
`14`	`14`
`15`		`-class Response(object):`
	`15`	`+class ApisResponse(object):`
`16`	`16`	`def __init__(self, response):`
`17`	`17`	`self.response = response`
`18`	`18`