launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/turnip/fix-json-coding into lp:turnip.

Commit message:
Use cornice/pyramid/webtest's built-in JSON encoding/decoding support rather than adding an extra layer on top, which resulted in responses containing JSON dumps wrapped in a string.

Requested reviews:
  Canonical Launchpad Branches (canonical-launchpad-branches)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/fix-json-coding/+merge/252588

$ env -u http_proxy curl http://git.launchpad.dev:19417/repo/1/refs/heads/master; echo
"{\"refs/heads/master\": {\"object\": {\"sha1\": \"8cf3a27f3c4369afafe4eba427c7e19c39f4026b\", \"type\": \"commit\"}}}"

This is one level of coding too many; I should be getting the JSON back directly here, rather than a JSON-encoded string containing a JSON-encoded dict.

This branch makes things simultaneously simpler and more correct by using cornice/pyramid/webtest's built-in JSON encoding/decoding support instead.
-- 
Your team Launchpad code reviewers is subscribed to branch lp:turnip.

=== modified file 'turnip/api/tests/test_api.py'
--- turnip/api/tests/test_api.py	2015-03-10 03:22:33 +0000
+++ turnip/api/tests/test_api.py	2015-03-11 14:26:03 +0000
@@ -2,7 +2,6 @@
 # -*- coding: utf-8 -*-
 from __future__ import print_function
 
-import json
 import os
 import unittest
 import uuid
@@ -32,15 +31,14 @@
 
     def get_ref(self, ref):
         resp = self.app.get('/repo/{}/{}'.format(self.repo_path, ref))
-        return json.loads(resp.json_body)
+        return resp.json
 
     def test_repo_create(self):
-        resp = self.app.post('/repo', json.dumps(
-            {'repo_path': self.repo_path}))
+        resp = self.app.post_json('/repo', {'repo_path': self.repo_path})
         self.assertEqual(resp.status_code, 200)
 
     def test_repo_delete(self):
-        self.app.post('/repo', json.dumps({'repo_path': self.repo_path}))
+        self.app.post_json('/repo', {'repo_path': self.repo_path})
         resp = self.app.delete('/repo/{}'.format(self.repo_path))
         self.assertEqual(resp.status_code, 200)
         self.assertFalse(os.path.exists(self.repo_store))
@@ -50,7 +48,7 @@
         ref = self.commit.get('ref')
         repo = RepoFactory(self.repo_store, num_commits=1, num_tags=1).build()
         resp = self.app.get('/repo/{}/refs'.format(self.repo_path))
-        body = json.loads(resp.json_body)
+        body = resp.json
 
         self.assertTrue(ref in body)
         self.assertTrue(self.tag.get('ref') in body)
@@ -68,7 +66,7 @@
         factory.add_tag(tag, tag_message, commit_oid)
 
         resp = self.app.get('/repo/{}/refs'.format(self.repo_path))
-        refs = json.loads(resp.json_body)
+        refs = resp.json
         self.assertEqual(len(refs.keys()), 1)
 
     def test_allow_unicode_refs(self):
@@ -80,7 +78,7 @@
         factory.add_tag(tag, tag_message, commit_oid)
 
         resp = self.app.get('/repo/{}/refs'.format(self.repo_path))
-        refs = json.loads(resp.json_body)
+        refs = resp.json
         self.assertEqual(len(refs.keys()), 2)
 
     def test_repo_get_ref(self):

=== modified file 'turnip/api/views.py'
--- turnip/api/views.py	2015-03-10 03:22:33 +0000
+++ turnip/api/views.py	2015-03-11 14:26:03 +0000
@@ -72,16 +72,14 @@
     @repo_path
     def collection_get(self, repo_path):
         try:
-            refs = store.get_refs(repo_path)
+            return store.get_refs(repo_path)
         except GitError:
             return exc.HTTPNotFound()  # 404
-        return json.dumps(refs, ensure_ascii=False)
 
     @repo_path
     def get(self, repo_path):
         ref = 'refs/' + self.request.matchdict['ref']
         try:
-            ref = store.get_ref(repo_path, ref)
+            return store.get_ref(repo_path, ref)
         except GitError:
             return exc.HTTPNotFound()
-        return json.dumps(ref, ensure_ascii=False)