REST: A check must specify a state

The Ozlabs crew noticed that a check without a state caused a
KeyError in data['state']. Mark state as mandatory, check for
it, and add a test.

Reported-by: Russell Currey <ruscur@russell.cc>
Reported-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>

diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
index e3ba69c5c64fe79627fcfe1f49846708ab9ff25a..724b05ebf1b3a0ea9a4910c77dbcd8e422cc44d3 100644 (file)
--- a/docs/api/schemas/latest/patchwork.yaml
+++ b/docs/api/schemas/latest/patchwork.yaml
@@ -1316,6 +1316,8 @@ components:
           nullable: true
     CheckCreate:
       type: object
+      required:
+       - state
       properties:
         state:
           title: State

diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index 7d3486387ede8730abf3d1ac31951abd187373f9..5e2f5e4ddc745cb37dac4ca1a3a787ceeb013eae 100644 (file)
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -1319,6 +1319,8 @@ components:
           nullable: true
     CheckCreate:
       type: object
+      required:
+       - state
       properties:
         state:
           title: State

diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml
index 11e3ae30adc04c7f1b26cba5c3e2c202c1bcd85a..02f3a1561b7bbcc50b03c3b5cce74e3f93fbb747 100644 (file)
--- a/docs/api/schemas/v1.0/patchwork.yaml
+++ b/docs/api/schemas/v1.0/patchwork.yaml
@@ -1311,6 +1311,8 @@ components:
           nullable: true
     CheckCreate:
       type: object
+      required:
+       - state
       properties:
         state:
           title: State

diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml
index 4e81ac33d9b2a5e7dcc29d4e5b30964d15cf4526..0c086edaa776414f2f683c60368c36f75e7cf757 100644 (file)
--- a/docs/api/schemas/v1.1/patchwork.yaml
+++ b/docs/api/schemas/v1.1/patchwork.yaml
@@ -1316,6 +1316,8 @@ components:
           nullable: true
     CheckCreate:
       type: object
+      required:
+       - state
       properties:
         state:
           title: State

diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 4d2181d0a04b1f79445e9d9f404bd7792f5c18fe..07d7cb9a13db0e2042e11460db6ed5cdd251944f 100644 (file)
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -12,6 +12,7 @@ from rest_framework.generics import RetrieveAPIView
 from rest_framework.serializers import CurrentUserDefault
 from rest_framework.serializers import HiddenField
 from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.serializers import ValidationError
 
 from patchwork.api.base import CheckHyperlinkedIdentityField
 from patchwork.api.base import MultipleFieldLookupMixin
@@ -36,6 +37,9 @@ class CheckSerializer(HyperlinkedModelSerializer):
     user = UserSerializer(default=CurrentUserDefault())
 
     def run_validation(self, data):
+        if 'state' not in data or data['state'] == '':
+            raise ValidationError({'state': ["A check must have a state."]})
+
         for val, label in Check.STATE_CHOICES:
             if label != data['state']:
                 continue

diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py
index 1cfdff6e757b7f32bd55d3ca12dbd4b28c7ab503..24451aba09adee3d3e8098458e95f38407905c96 100644 (file)
--- a/patchwork/tests/api/test_check.py
+++ b/patchwork/tests/api/test_check.py
@@ -151,6 +151,23 @@ class TestCheckAPI(utils.APITestCase):
         self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
         self.assertEqual(0, Check.objects.all().count())
 
+    @utils.store_samples('check-create-error-missing-state')
+    def test_create_missing_state(self):
+        """Create a check using invalid values.
+
+        Ensure we handle the state being absent.
+        """
+        check = {
+            'target_url': 'http://t.co',
+            'description': 'description',
+            'context': 'context',
+        }
+
+        self.client.force_authenticate(user=self.user)
+        resp = self.client.post(self.api_url(), check)
+        self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+        self.assertEqual(0, Check.objects.all().count())
+
     @utils.store_samples('check-create-error-not-found')
     def test_create_invalid_patch(self):
         """Ensure we handle non-existent patches."""