fix(controller): properly serialize JSONField objects

JSONField is a finnicky thing. When serializing the object in the API,
it will just dump the object as a string. This ends in an invalid JSON
response. In the case of the Config object:

    {"owner": "bacongobbler", "app": "go", "values": "{\"HELLO\": \"world\"}", "memory": "{}", "cpu": "{}", "tags": "{}", ...}

This fix changes the JSON output to the following:

    {"owner": "bacongobbler", "app": "go", "values": {"HELLO": "world"}, "memory": {}, "cpu": {}, "tags": {}}

This is completely backwards compatible with previous requests. The
JSONSerializer will accept both strings and JSON objects in the request.

Author: Matthew Fisher

client/deis.py

@@ -1122,7 +1122,7 @@ def config_list(self, args):
         response = self._dispatch('get', "/api/apps/{}/config".format(app))
         if response.status_code == requests.codes.ok:  # @UndefinedVariable
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {} Config".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1177,7 +1177,7 @@ def config_set(self, args):
             version = response.headers['x-deis-release']
             self._logger.info("done, v{}\n".format(version))
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {}".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1223,7 +1223,7 @@ def config_unset(self, args):
             version = response.headers['x-deis-release']
             self._logger.info("done, v{}\n".format(version))
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {}".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1267,7 +1267,7 @@ def config_pull(self, args):
                 pass
         response = self._dispatch('get', "/api/apps/{}/config".format(app))
         if response.status_code == requests.codes.ok:  # @UndefinedVariable
-            config = json.loads(response.json()['values'])
+            config = response.json()['values']
             for k, v in config.items():
                 if interactive and raw_input("overwrite {} with {}? (y/N) ".format(k, v)) == 'y':
                     env_dict[k] = v
@@ -1542,9 +1542,9 @@ def write(d):
                 self._logger.info(("{k:<" + str(width) + "} {v}").format(**locals()))
 
         self._logger.info("\n--- Memory")
-        write(json.loads(config.get('memory', '{}')))
+        write(config.get('memory', '{}'))
         self._logger.info("\n--- CPU")
-        write(json.loads(config.get('cpu', '{}')))
+        write(config.get('cpu', '{}'))
 
     def ps(self, args):
         """
@@ -1740,7 +1740,7 @@ def tags_unset(self, args):
             raise ResponseError(response)
 
     def _print_tags(self, app, config):
-        items = json.loads(config['tags'])
+        items = config['tags']
         self._logger.info("=== {} Tags".format(app))
         if len(items) == 0:
             self._logger.info('No tags defined')

controller/api/serializers.py

@@ -4,6 +4,7 @@
 
 from __future__ import unicode_literals
 
+import json
 import re
 
 from django.conf import settings
@@ -32,6 +33,18 @@ def from_native(self, data):
         return serializers.SlugRelatedField.from_native(self, data)
 
 
+class JSONFieldSerializer(serializers.WritableField):
+    def to_native(self, obj):
+        return obj
+
+    def from_native(self, value):
+        try:
+            val = json.loads(value)
+        except TypeError:
+            val = value
+        return val
+
+
 class UserSerializer(serializers.ModelSerializer):
     """Serialize a User model."""
 
@@ -64,6 +77,7 @@ class ClusterSerializer(serializers.ModelSerializer):
     """Serialize a :class:`~api.models.Cluster` model."""
 
     owner = serializers.Field(source='owner.username')
+    options = JSONFieldSerializer(source='options', required=False)
 
     class Meta:
         """Metadata options for a :class:`ClusterSerializer`."""
@@ -98,6 +112,7 @@ class BuildSerializer(serializers.ModelSerializer):
 
     owner = serializers.Field(source='owner.username')
     app = serializers.SlugRelatedField(slug_field='id')
+    procfile = JSONFieldSerializer(source='procfile', required=False)
 
     class Meta:
         """Metadata options for a :class:`BuildSerializer`."""
@@ -110,14 +125,10 @@ class ConfigSerializer(serializers.ModelSerializer):
 
     owner = serializers.Field(source='owner.username')
     app = serializers.SlugRelatedField(slug_field='id')
-    values = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('values'), required=False)
-    memory = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('memory'), required=False)
-    cpu = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('cpu'), required=False)
-    tags = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('tags'), required=False)
+    values = JSONFieldSerializer(source='values', required=False)
+    memory = JSONFieldSerializer(source='memory', required=False)
+    cpu = JSONFieldSerializer(source='cpu', required=False)
+    tags = JSONFieldSerializer(source='tags', required=False)
 
     class Meta:
         """Metadata options for a :class:`ConfigSerializer`."""
@@ -185,6 +196,7 @@ class AppSerializer(serializers.ModelSerializer):
     id = serializers.SlugField(default=utils.generate_app_name)
     cluster = serializers.SlugRelatedField(slug_field='id')
     url = serializers.Field(source='url')
+    structure = JSONFieldSerializer(source='structure', required=False)
 
     class Meta:
         """Metadata options for a :class:`AppSerializer`."""
@@ -203,14 +215,6 @@ def validate_id(self, attrs, source):
             raise serializers.ValidationError("App IDs cannot be 'deis'")
         return attrs
 
-    def validate_structure(self, attrs, source):
-        """
-        Check that the structure JSON dict has non-negative ints as its values.
-        """
-        value = attrs[source]
-        models.validate_app_structure(value)
-        return attrs
-
 
 class ContainerSerializer(serializers.ModelSerializer):
     """Serialize a :class:`~api.models.Container` model."""

controller/api/tests/test_app.py

@@ -10,12 +10,12 @@
 import os.path
 
 from django.test import TestCase
-
 from django.conf import settings
 
+from api.models import App
 
-class AppTest(TestCase):
 
+class AppTest(TestCase):
     """Tests creation of applications"""
 
     fixtures = ['tests.json']
@@ -135,6 +135,23 @@ def test_app_errors(self):
             response = self.client.get(url)
             self.assertEquals(response.status_code, 404)
 
+    def test_app_structure_is_valid_json(self):
+        """Application structures should be valid JSON objects."""
+        url = '/api/apps'
+        body = {'cluster': 'autotest'}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 201)
+        app_id = response.data['id']
+        self.assertIn('structure', response.data)
+        self.assertEqual(response.data['structure'], {})
+        app = App.objects.get(id=app_id)
+        app.structure = {'web': 1}
+        app.save()
+        url = '/api/apps/{}'.format(app_id)
+        response = self.client.get(url)
+        self.assertIn('structure', response.data)
+        self.assertEqual(response.data['structure'], {"web": 1})
+
     def test_admin_can_manage_other_apps(self):
         """Administrators of Deis should be able to manage all applications.
         """

controller/api/tests/test_cluster.py

@@ -42,7 +42,7 @@ def test_cluster(self):
         self.assertIn('auth', response.data)
         self.assertIn('options', response.data)
         self.assertEqual(response.data['hosts'], 'host1,host2')
-        self.assertEqual(json.loads(response.data['options']), {'key': 'val'})
+        self.assertEqual(response.data['options'], {'key': 'val'})
         response = self.client.get('/api/clusters')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data['results']), 1)
@@ -68,7 +68,7 @@ def test_cluster(self):
         response = self.client.patch(url, json.dumps(body), content_type='application/json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['hosts'], new_hosts)
-        self.assertEqual(json.loads(response.data['options']), new_options)
+        self.assertEqual(response.data['options'], new_options)
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)