Merge pull request #4071 from Joshua-Anderson/cpu-int-fix

fix(controller): convert config to correct types

Author: mboersma

controller/api/serializers.py

@@ -24,17 +24,58 @@
 
 
 class JSONFieldSerializer(serializers.Field):
+    """
+    A Django REST framework serializer for JSON data.
+    """
+
     def to_representation(self, obj):
+        """Serialize the field's JSON data, for read operations."""
         return obj
 
     def to_internal_value(self, data):
+        """Deserialize the field's JSON data, for write operations."""
         try:
             val = json.loads(data)
         except TypeError:
             val = data
         return val
 
 
+class JSONIntFieldSerializer(JSONFieldSerializer):
+    """
+    A JSON serializer that coerces its data to integers.
+    """
+
+    def to_internal_value(self, data):
+        """Deserialize the field's JSON integer data."""
+        field = super(JSONIntFieldSerializer, self).to_internal_value(data)
+
+        for k, v in field.viewitems():
+            if v is not None:  # NoneType is used to unset a value
+                try:
+                    field[k] = int(v)
+                except ValueError:
+                    field[k] = v
+                    # Do nothing, the validator will catch this later
+        return field
+
+
+class JSONStringFieldSerializer(JSONFieldSerializer):
+    """
+    A JSON serializer that coerces its data to strings.
+    """
+
+    def to_internal_value(self, data):
+        """Deserialize the field's JSON string data."""
+        field = super(JSONStringFieldSerializer, self).to_internal_value(data)
+
+        for k, v in field.viewitems():
+            if v is not None:  # NoneType is used to unset a value
+                field[k] = unicode(v)
+
+        return field
+
+
 class ModelSerializer(serializers.ModelSerializer):
 
     uuid = serializers.ReadOnlyField()
@@ -129,10 +170,10 @@ class ConfigSerializer(ModelSerializer):
 
     app = serializers.SlugRelatedField(slug_field='id', queryset=models.App.objects.all())
     owner = serializers.ReadOnlyField(source='owner.username')
-    values = JSONFieldSerializer(required=False)
-    memory = JSONFieldSerializer(required=False)
-    cpu = JSONFieldSerializer(required=False)
-    tags = JSONFieldSerializer(required=False)
+    values = JSONStringFieldSerializer(required=False)
+    memory = JSONStringFieldSerializer(required=False)
+    cpu = JSONIntFieldSerializer(required=False)
+    tags = JSONStringFieldSerializer(required=False)
     created = serializers.DateTimeField(format=settings.DEIS_DATETIME_FORMAT, read_only=True)
     updated = serializers.DateTimeField(format=settings.DEIS_DATETIME_FORMAT, read_only=True)
 

controller/api/tests/test_config.py

@@ -114,7 +114,7 @@ def test_response_data(self):
         response = self.client.post('/v1/apps', json.dumps(body),
                                     content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
-        url = "/v1/apps/test/config".format()
+        url = "/v1/apps/test/config"
         # set an initial config value
         body = {'values': json.dumps({'PORT': '5000'})}
         response = self.client.post(url, json.dumps(body), content_type='application/json',
@@ -132,6 +132,38 @@ def test_response_data(self):
         }
         self.assertDictContainsSubset(expected, response.data)
 
+    @mock.patch('requests.post', mock_import_repository_task)
+    def test_response_data_types_converted(self):
+        """Test that config data is converted into the correct type."""
+        body = {'id': 'test'}
+        response = self.client.post('/v1/apps', json.dumps(body),
+                                    content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        url = "/v1/apps/test/config"
+
+        body = {'values': json.dumps({'PORT': 5000}), 'cpu': json.dumps({'web': '1024'})}
+        response = self.client.post(url, json.dumps(body), content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 201)
+        for key in response.data:
+            self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'values', 'memory',
+                                'cpu', 'tags'])
+        expected = {
+            'owner': self.user.username,
+            'app': 'test',
+            'values': {'PORT': '5000'},
+            'memory': {},
+            'cpu': {'web': 1024},
+            'tags': {}
+        }
+        self.assertDictContainsSubset(expected, response.data)
+
+        body = {'cpu': json.dumps({'web': 'this will fail'})}
+        response = self.client.post(url, json.dumps(body), content_type='application/json',
+                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 400)
+        self.assertIn('CPU shares must be an integer', response.data['cpu'])
+
     @mock.patch('requests.post', mock_import_repository_task)
     def test_config_set_same_key(self):
         """
@@ -185,7 +217,7 @@ def test_config_set_unicode(self):
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 201)
         self.assertIn('INTEGER', response.data['values'])
-        self.assertEqual(response.data['values']['INTEGER'], 1)
+        self.assertEqual(response.data['values']['INTEGER'], '1')
 
     @mock.patch('requests.post', mock_import_repository_task)
     def test_config_str(self):
@@ -333,7 +365,7 @@ def test_limit_cpu(self):
         self.assertIn('cpu', response.data)
         cpu = response.data['cpu']
         self.assertIn('web', cpu)
-        self.assertEqual(cpu['web'], '1024')
+        self.assertEqual(cpu['web'], 1024)
         # set an additional value
         body = {'cpu': json.dumps({'worker': '512'})}
         response = self.client.post(url, json.dumps(body), content_type='application/json',
@@ -343,19 +375,19 @@ def test_limit_cpu(self):
         self.assertNotEqual(limit1['uuid'], limit2['uuid'])
         cpu = response.data['cpu']
         self.assertIn('worker', cpu)
-        self.assertEqual(cpu['worker'], '512')
+        self.assertEqual(cpu['worker'], 512)
         self.assertIn('web', cpu)
-        self.assertEqual(cpu['web'], '1024')
+        self.assertEqual(cpu['web'], 1024)
         # read the limit again
         response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
         self.assertEqual(response.status_code, 200)
         limit3 = response.data
         self.assertEqual(limit2, limit3)
         cpu = response.data['cpu']
         self.assertIn('worker', cpu)
-        self.assertEqual(cpu['worker'], '512')
+        self.assertEqual(cpu['worker'], 512)
         self.assertIn('web', cpu)
-        self.assertEqual(cpu['web'], '1024')
+        self.assertEqual(cpu['web'], 1024)
         # unset a value
         body = {'memory': json.dumps({'worker': None})}
         response = self.client.post(url, json.dumps(body), content_type='application/json',