[PATCH 2/2] REST: A check must specify a state

[Daniel Axtens]

Applied with some minor tweaks.

> 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 at russell.cc>
> Reported-by: Jeremy Kerr <jk at ozlabs.org>
> Signed-off-by: Daniel Axtens <dja at axtens.net>
>
> ---
>
> Also needs to go to stable.
> ---
>  docs/api/schemas/latest/patchwork.yaml |  2 ++
>  docs/api/schemas/patchwork.j2          |  2 ++
>  docs/api/schemas/v1.0/patchwork.yaml   |  2 ++
>  docs/api/schemas/v1.1/patchwork.yaml   |  2 ++
>  patchwork/api/check.py                 |  4 ++++
>  patchwork/tests/api/test_check.py      | 17 +++++++++++++++++
>  6 files changed, 29 insertions(+)
>
> diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
> index e3ba69c5c64f..724b05ebf1b3 100644
> --- 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 7d3486387ede..5e2f5e4ddc74 100644
> --- 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 11e3ae30adc0..02f3a1561b7b 100644
> --- 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 4e81ac33d9b2..0c086edaa776 100644
> --- 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 4d2181d0a04b..2c3fe445872e 100644
> --- a/patchwork/api/check.py
> +++ b/patchwork/api/check.py
> @@ -7,6 +7,7 @@ from django.http import Http404
>  from django.http.request import QueryDict
>  from django.shortcuts import get_object_or_404
>  from rest_framework.exceptions import PermissionDenied
> +from rest_framework.exceptions import ValidationError
>  from rest_framework.generics import ListCreateAPIView
>  from rest_framework.generics import RetrieveAPIView
>  from rest_framework.serializers import CurrentUserDefault
> @@ -36,6 +37,9 @@ class CheckSerializer(HyperlinkedModelSerializer):
>      user = UserSerializer(default=CurrentUserDefault())
>  
>      def run_validation(self, data):
> +        if 'state' not in data:
> +            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 1cfdff6e757b..24451aba09ad 100644
> --- 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."""
> -- 
> 2.19.1