Merge lp:~elopio/cloudspacesclient/return_json into lp:cloudspacesclient

[Leo Arias]

Refactor the client to return the json after each request.

[lp:~elopio/cloudspacesclient/return_json]

27. By Leo Arias on 2013-11-26

Make a better check of the returned json.

[Łukasz Czyżykowski (lukasz-czyzykowski)]

LGTM

review: Approve

[lp:~elopio/cloudspacesclient/return_json]

28. By Leo Arias on 2013-11-27

Merged with trunk.

=== modified file 'src/cloudspacesclient/client.py'
--- src/cloudspacesclient/client.py	2013-11-21 04:32:56 +0000
+++ src/cloudspacesclient/client.py	2013-11-27 06:07:02 +0000
@@ -45,7 +45,7 @@
         if not response.ok:
             raise APIException(
                 "{0}: {1}".format(response.status_code, response.reason))
-        return response
+        return response.json()
 
     def _get_headers(self):
         return {
@@ -66,4 +66,4 @@
         if not response.ok:
             raise APIException(
                 "{0}: {1}".format(response.status_code, response.reason))
-        return response
+        return response.json()

=== modified file 'src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py'
--- src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py	2013-11-26 16:34:44 +0000
+++ src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py	2013-11-27 06:07:02 +0000
@@ -56,12 +56,10 @@
         return parser.parse(rfc3339_now)
 
     def test_get_user_representation(self):
-        response = self.cloudspaces_api_client.get_user_representation()
-        self.assertTrue(response.ok)
-        response_json = response.json()
+        user_rep = self.cloudspaces_api_client.get_user_representation()
         validictory.validate(
-            response_json, schemas.GET_USER_REPRESENTATION_RESPONSE_SCHEMA)
-        volumes = response_json.get('volumes')
+            user_rep, schemas.GET_USER_REPRESENTATION_RESPONSE_SCHEMA)
+        volumes = user_rep.get('volumes')
         # Assert that the volumes have just been created.
         self._assert_volumes_when_created(
             volumes, self.start_datetime, self._get_now_datetime())
@@ -93,32 +91,31 @@
             resource_path, '{0}metadata/{1}'.format(path, node_id))
 
     def test_get_root_metadata(self):
-        user_rep = self.cloudspaces_api_client.get_user_representation().json()
+        user_rep = self.cloudspaces_api_client.get_user_representation()
         root_volume = self._get_root_volume(user_rep.get('volumes'))
         node_id = root_volume.get('node_id')
         filename = os.path.split(root_volume.get('name'))[-1]
-        response = self.cloudspaces_api_client.get_metadata(node_id)
-        self.assertTrue(response.ok)
-        response_json = response.json()
+
+        metadata = self.cloudspaces_api_client.get_metadata(node_id)
         validictory.validate(
-            response_json, schemas.GET_METADATA_RESPONSE_SCHEMA)
-        self._assert_normal_folder_keys_not_in_root(response_json)
-        self._assert_content_path(node_id, response_json.get('content_path'))
+            metadata, schemas.GET_METADATA_RESPONSE_SCHEMA)
+        self._assert_normal_folder_keys_not_in_root(metadata)
+        self._assert_content_path(node_id, metadata.get('content_path'))
         # TODO Check that the contents are not present because the folder is
         # empty. --elopio - 2013-11-25
-        self.assertTrue(response_json.get('is_folder'))
-        self.assertFalse(response_json.get('is_deleted'))
-        self.assertTrue(response_json.get('is_root'))
+        self.assertTrue(metadata.get('is_folder'))
+        self.assertFalse(metadata.get('is_deleted'))
+        self.assertTrue(metadata.get('is_root'))
         # TODO The spec doesn't include it in the filename in the examples.
         # --elopio - 2013-11-25
-        self.assertEqual(response_json.get('filename'), filename)
-        self.assertEqual(response_json.get('node_id'), node_id)
+        self.assertEqual(metadata.get('filename'), filename)
+        self.assertEqual(metadata.get('node_id'), node_id)
         # TODO The spec says the path shouldn't be returned on the root
         # metadata, but I think it's useful. Should we change the spec?
         # --elopio - 2013-11-25
         self.assertEqual(
-            response_json.get('path'), self.test_server.get_root_path())
-        self._assert_resource_path(node_id, response_json.get('resource_path'))
+            metadata.get('path'), self.test_server.get_root_path())
+        self._assert_resource_path(node_id, metadata.get('resource_path'))
         # TODO Check the volume_id field. --elopio - 2013-11-25
 
     def _get_root_volume(self, volumes):

=== modified file 'src/cloudspacesclient/tests/unit/test_client.py'
--- src/cloudspacesclient/tests/unit/test_client.py	2013-11-20 05:57:50 +0000
+++ src/cloudspacesclient/tests/unit/test_client.py	2013-11-27 06:07:02 +0000
@@ -17,6 +17,7 @@
 
 """Unit tests for the Cloudspaces API client."""
 
+import json
 import unittest
 
 import mock
@@ -30,17 +31,19 @@
     def setUp(self):
         super(CloudspacesAPIClientTestCase, self).setUp()
         self.api_client = cloudspacesclient.CloudspacesAPIClient(
-            'dummy url', 'dummy auth')
+            'http://example.com/cloudspaces/', 'test auth')
+        self.request_headers = {
+            'Content-Type': 'application/json',
+            'Accept': 'application/json'
+        }
 
-    def test_get_user_representation_should_return_request(self):
+    def test_get_user_representation_should_return_json(self):
         with mock.patch('requests.get') as get_mock:
             get_mock.return_value = utils.RequestResponseMock(
-                ok=True, status_code=200, content='test content')
+                ok=True, content='{"test": "content"}')
             response = self.api_client.get_user_representation()
 
-        self.assertTrue(response.ok)
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.content, 'test content')
+        self.assertEqual(response.get('test'), 'content')
 
     def test_get_user_representation_failure(self):
         with mock.patch('requests.get') as get_mock:
@@ -50,3 +53,40 @@
                 self.api_client.get_user_representation()
 
         self.assertEqual(str(e.exception), 'test error code: test reason')
+
+    def test_get_user_representation_request(self):
+        with mock.patch('requests.get') as get_mock:
+            get_mock.return_value = utils.RequestResponseMock(
+                ok=True, content=json.dumps("{'test': 'content'}"))
+            self.api_client.get_user_representation()
+
+        get_mock.assert_called_once_with(
+            'http://example.com/cloudspaces/', auth='test auth',
+            headers=self.request_headers)
+
+    def test_get_metadata_should_return_json(self):
+        with mock.patch('requests.get') as get_mock:
+            get_mock.return_value = utils.RequestResponseMock(
+                ok=True, content=json.dumps("{'test': 'content'}"))
+            response = self.api_client.get_metadata('dummy node')
+
+        self.assertEqual(response, "{'test': 'content'}")
+
+    def test_get_metadata_failure(self):
+        with mock.patch('requests.get') as get_mock:
+            get_mock.return_value = utils.RequestResponseMock(
+                ok=False, status_code='test error code', reason='test reason')
+            with self.assertRaises(cloudspacesclient.APIException) as e:
+                self.api_client.get_metadata('dummy node')
+
+        self.assertEqual(str(e.exception), 'test error code: test reason')
+
+    def test_get_metadata_request(self):
+        with mock.patch('requests.get') as get_mock:
+            get_mock.return_value = utils.RequestResponseMock(
+                ok=True, content=json.dumps("{'test': 'content'}"))
+            self.api_client.get_metadata('testnodeid')
+
+        get_mock.assert_called_once_with(
+            'http://example.com/cloudspaces/metadata/testnodeid',
+            auth='test auth', headers=self.request_headers)

=== modified file 'src/cloudspacesclient/tests/unit/utils.py'
--- src/cloudspacesclient/tests/unit/utils.py	2013-11-20 05:06:14 +0000
+++ src/cloudspacesclient/tests/unit/utils.py	2013-11-27 06:07:02 +0000
@@ -18,12 +18,23 @@
 """Utilities used in multiple unit tests."""
 
 
-class RequestResponseMock(object):
+from requests import models
+
+
+class RequestResponseMock(models.Response):
 
     def __init__(self, ok=True, status_code=200, reason='OK',
                  content='mock content'):
         super(RequestResponseMock, self).__init__()
-        self.ok = ok
+        self._ok = ok
         self.status_code = status_code
         self.reason = reason
-        self.content = content
+        self._content = content
+
+    @property
+    def content(self):
+        return self._content
+
+    @property
+    def ok(self):
+        return self._ok