Merge branch 'bottint143' into 'master'

Treating 404 differently based on the reasons and multiple fixes

See merge request !30

setra_client/client.py

@@ -18,6 +18,7 @@ from setra_client.models import (
     AbwOrder,
     AbwOrderErrors,
     ResponseStatusEnum,
+    Voucher,
 )
 
 logger = logging.getLogger(__name__)
@@ -43,6 +44,10 @@ def merge_dicts(*dicts):
     return combined
 
 
+class IncorrectPathError(Exception):
+    pass
+
+
 class SetraEndpoints:
     def __init__(self,
                  url,
@@ -241,6 +246,8 @@ class SetraClient(object):
              params=None,
              return_response=True,
              **kwargs):
+        if method_name == 'GET':
+            return_response = False
         headers = self._build_request_headers(headers)
         if params is None:
             params = {}
@@ -255,10 +262,30 @@ class SetraClient(object):
                                  **kwargs)
         if r.status_code in (500, 400, 401):
             logger.warning('Got HTTP %d: %r', r.status_code, r.content)
+        elif r.status_code == 404 and method_name == 'GET':
+            try:
+                data = r.json()
+                if 'detail' in data and 'Not found' in data['detail']:
+                    return None
+                else:
+                    raise IncorrectPathError
+            except:
+                data = r.text
+                if 'Not found' in data:
+                    return None
+                else:
+                    raise IncorrectPathError
         if return_response:
             return r
         r.raise_for_status()
-        return r.json()  # Note: krasjer her, dersom man får text og ikke json i requesten, og return_response=false
+
+        # If we got a valid answer, return Json if there is a json in it
+        # Otherwise, return just the text
+        try:
+            return_data = r.json()
+        except:
+            return_data = r.text
+        return return_data
 
     def get(self, url, **kwargs):
         return self.call('GET', url, **kwargs)
@@ -305,9 +332,7 @@ class SetraClient(object):
         }
 
         url = self.urls.batch()
-        response = self.get(url, params=params)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url, params=params)
         if self.return_objects:
             if isinstance(data, list):
                 return [OutputBatch(**item) for item in data]
@@ -322,10 +347,8 @@ class SetraClient(object):
         """
 
         url = self.urls.batch(str(batch_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
-        if self.return_objects:
+        data = self.get(url)
+        if data and self.return_objects:
             return OutputBatch(**data)
         else:
             return data
@@ -338,8 +361,8 @@ class SetraClient(object):
             vouch_id = str(vouch_id)
 
         url = self.urls.voucher(vouch_id)
-        response = self.get(url)
-        return response.json()
+        data = self.get(url)
+        return data
 
     def get_transaction(self, trans_id: int = None):
         """
@@ -349,8 +372,8 @@ class SetraClient(object):
             trans_id = str(trans_id)
 
         url = self.urls.transaction(trans_id)
-        response = self.get(url)
-        return response.json()
+        data = self.get(url)
+        return data
 
     def post_new_batch(self, batchdata: InputBatch):
         """
@@ -408,13 +431,13 @@ class SetraClient(object):
         from SETRA
         """
         url = self.urls.batch_complete(batch_id)
-        response = self.get(url)
-        return self.object_or_data(CompleteBatch, response.json())
+        data = self.get(url)
+        return self.object_or_data(CompleteBatch, data)
 
     def get_batch_errors(self, batch_id: str):
         url = self.urls.batch_error(batch_id)
-        response = self.get(url)
-        return self.object_or_data(BatchErrors, response.json())
+        data = self.get(url)
+        return self.object_or_data(BatchErrors, data)
 
     def get_parameters(self, interface: str = None):
         """Make a GET request to the parameters endpoint"""
@@ -422,8 +445,8 @@ class SetraClient(object):
         queryparams = None
         if interface:
             queryparams = {"interface": interface}
-        response = self.get(url, params=queryparams)
-        return self.object_or_data(Parameter, response.json())
+        data = self.get(url, params=queryparams)
+        return self.object_or_data(Parameter, data)
 
     # Order ("Sotra") functions:
 
@@ -432,9 +455,7 @@ class SetraClient(object):
         GETs one order object
         """
         url = self.urls.order(str(order_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url)
         if self.return_objects:
             return Order(**data)
         else:
@@ -445,9 +466,7 @@ class SetraClient(object):
         GETs one detail object
         """
         url = self.urls.detail(str(detail_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url)
         if self.return_objects:
             return Detail(**data)
         else:
@@ -458,19 +477,15 @@ class SetraClient(object):
         GETs a list of all orders, without detail objects
         """
         url = self.urls.order()
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
-        return self.object_or_data(Order, response.json())
+        data = self.get(url)
+        return self.object_or_data(Order, data)
 
     def get_details_in_order(self, order_id: str) -> Union[List[Detail], dict]:
         """
         GETs list of all detail objects belonging to an order
         """
         url = self.urls.details_in_order(str(order_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url)
         if self.return_objects:
             if isinstance(data, list):
                 return [Detail(**item) for item in data]
@@ -484,9 +499,7 @@ class SetraClient(object):
         GETs one order, with all detail objects
         """
         url = self.urls.order_complete(str(order_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url)
         if self.return_objects:
             return Order(**data)
         else:
@@ -497,9 +510,7 @@ class SetraClient(object):
         GETs one abworder, with all order and detail objects
         """
         url = self.urls.abw_order_complete(str(abw_order_id))
-        response = self.get(url)
-        data = response.json()
-        response.raise_for_status()
+        data = self.get(url)
         if self.return_objects:
             return AbwOrder(**data)
         else:

tests/test_client.py

@@ -8,6 +8,7 @@ from requests import HTTPError
 
 from setra_client.client import SetraClient
 from setra_client.client import SetraEndpoints
+from setra_client.client import IncorrectPathError
 from setra_client.models import (
     CompleteBatch,
     BatchErrors,
@@ -66,10 +67,8 @@ def test_get_successful_batch_with_json_content(client, requests_mock, baseurl):
 
     url = SetraEndpoints(baseurl).batch(batch_id="3")  # https://localhost/api/batch/3
     requests_mock.get(url, json={"foo": "bar"}, status_code=200)
-
     response = client.call(method_name="GET", url=url)
-    assert response.status_code == 200
-    assert response.json() == {"foo": "bar"}
+    assert response == {"foo": "bar"}
 
 
 def test_get_successful_batch_with_text_content(client, requests_mock, baseurl):
@@ -79,18 +78,16 @@ def test_get_successful_batch_with_text_content(client, requests_mock, baseurl):
     requests_mock.get(url, text="some content", status_code=200)
 
     response = client.call(method_name="GET", url=url)
-    assert response.status_code == 200
-    assert response.text == "some content"
+    assert response == "some content"
 
 
 def test_get_failing_batch_with_json_content(client, batch_url, requests_mock, baseurl):
     """A failing GET call with 404 should pass through the same response from the client, and return HTTP 404,
     and the request with json content"""
     requests_mock.get(batch_url, json={"a": "b"}, status_code=404)
-
-    resp = client.call(method_name="GET", url=batch_url)
-    assert resp.status_code == 404
-    assert resp.json() == {"a": "b"}
+    
+    with pytest.raises(IncorrectPathError):
+        resp = client.call(method_name="GET", url=batch_url)
 
 
 def test_get_failing_batch_with_text_content(client, batch_url, requests_mock, baseurl):
@@ -98,9 +95,8 @@ def test_get_failing_batch_with_text_content(client, batch_url, requests_mock, b
     and the request with json content"""
     requests_mock.get(batch_url, text="some content", status_code=404)
 
-    resp = client.call(method_name="GET", url=batch_url)
-    assert resp.status_code == 404
-    assert resp.text == "some content"
+    with pytest.raises(IncorrectPathError):
+        resp = client.call(method_name="GET", url=batch_url)
 
 
 def test_get_failing_batch_without_return_response_text(
@@ -110,12 +106,10 @@ def test_get_failing_batch_without_return_response_text(
     and return HTTP 404, and the request with text content"""
     requests_mock.get(batch_url, text="some content", status_code=404)
 
-    with pytest.raises(HTTPError) as err:
+    with pytest.raises(IncorrectPathError) as err:
         client.call(method_name="GET", url=batch_url, return_response=False)
 
-    assert err.type == requests.exceptions.HTTPError
-    assert err.value.response.status_code == 404
-    assert err.value.response.text == "some content"
+    assert err.type == IncorrectPathError
 
 
 def test_get_failing_batch_without_return_response_json(
@@ -125,12 +119,10 @@ def test_get_failing_batch_without_return_response_json(
     and return HTTP 404, and the request with json content"""
     requests_mock.get(batch_url, json={"a": "b"}, status_code=404)
 
-    with pytest.raises(HTTPError) as err:
+    with pytest.raises(IncorrectPathError) as err:
         client.call(method_name="GET", url=batch_url, return_response=False)
 
-    assert err.type == requests.exceptions.HTTPError
-    assert err.value.response.status_code == 404
-    assert err.value.response.json() == {"a": "b"}
+    assert err.type == IncorrectPathError
 
 
 def test_get_failing_batch_without_return_response2(
@@ -139,12 +131,12 @@ def test_get_failing_batch_without_return_response2(
     """A failing GET call with 404 returning text content, with return_response=False
     should raise for HTTPError from the client
     (because we expect json from setra, and it will give error with text content)"""
-    requests_mock.get(batch_url, text="some content", status_code=200)
+    requests_mock.get(batch_url, text="some content", status_code=404)
 
-    with pytest.raises(requests.exceptions.JSONDecodeError) as err:
-        client.call(method_name="GET", url=batch_url, return_response=False)
+    with pytest.raises(IncorrectPathError) as err:
+        client.call(method_name="GET", url=batch_url)
 
-    assert err.type == requests.exceptions.JSONDecodeError
+    assert err.type == IncorrectPathError
 
 
 # Test post method
@@ -188,12 +180,10 @@ def test_header_replacement(client_with_a_header, batch_url, requests_mock, base
         headers={"content-type": "replaced"},
     )
 
-    response = client_with_a_header.call(
+    data = client_with_a_header.call(
         method_name="GET", url=batch_url, headers={"content-type": "replaced"}
     )
-    assert response.status_code == 200
-    assert response.text == "some content"
-    assert response.headers == {"content-type": "replaced"}
+    assert data == "some content"
 
 
 def test_header_replacement2(client_with_a_header, batch_url, requests_mock, baseurl):
@@ -208,12 +198,10 @@ def test_header_replacement2(client_with_a_header, batch_url, requests_mock, bas
         headers={"content-type": "replaced", "key": "val"},
     )
 
-    response = client_with_a_header.call(
+    data = client_with_a_header.call(
         method_name="GET", url=batch_url, headers={"content-type": "replaced"}
     )
-    assert response.status_code == 200
-    assert response.text == "some content"
-    assert response.headers == {"content-type": "replaced", "key": "val"}
+    assert data == "some content"
 
 
 # Test get_batches method
@@ -295,14 +283,14 @@ def test_successfully_getting_single_voucher_with_alphanumeric_voucherid(
     response = client.get_voucher("abcd123efg")  # using alphanum string as voucherid
     assert response == {"foo": "bar"}
 
-
+#CLARIFY
 def test_failing_to_get_all_vouchers(client, requests_mock, baseurl):
-    """A failing GET all vouchers call should still return json"""
+    """A failing GET all vouchers call should still return json""" #REALLY?
     url = SetraEndpoints(baseurl).voucher()
-    requests_mock.get(url, json={"error": "some json error message"}, status_code=404)
+    requests_mock.get(url, json={'detail': 'Not found.'}, status_code=404)
 
     response = client.get_voucher()
-    assert response == {"error": "some json error message"}
+    assert response == None
 
 
 # Test get_transaction method
@@ -320,10 +308,10 @@ def test_successfully_getting_single_transaction(client, requests_mock, baseurl)
 def test_failing_to_get_all_transactions(client, requests_mock, baseurl):
     """A failing GET all vouchers call should still return json"""
     url = SetraEndpoints(baseurl).transaction()
-    requests_mock.get(url, json={"error": "some json error message"}, status_code=404)
+    requests_mock.get(url, json={'detail': 'Not found.'}, status_code=404)
 
     response = client.get_transaction()
-    assert response == {"error": "some json error message"}
+    assert response == None
 
 
 # Test post_new_batch method