2
\$\begingroup\$

Context

I am trying to get some practice with Django and Django REST framework.
As a toy example I want to set up a server to which I can POST results of automated test runs.
In the simplest version, a test result contains the following information:

  • a testcase id, which is unique and will never change
  • a testcase name, which might change from time to time
  • a verdict, which may only take one of the predefined values "PASS", "FAIL" or "SKIPPED"

I came up with three different possible solutions. Each one is working, but I am not sure which one is the best approach since I am lacking experience.

Expected behavior

The implementation should fulfill the following requirements:

  • POST request containing any other verdict than PASS, FAIL or SKIPPED -> HTTP 400
  • POST request with a new and yet unused testcase id -> create and use new testcase instance
  • POST request with an existing testcase id but different testcase name-> retrieve existing testcase instance for this id and update the name in the database
  • GET request -> return list of all test results with testcase id, testcase name and verdict

Django Models

For the models I have the following code:

models.py:

from django.db import models


class Testcase(models.Model):
    uid = models.CharField(max_length=10, unique=True)
    name = models.CharField(max_length=50)

    def __str__(self):
        return f"{self.uid:<10} {self.name:<50}"


class Verdict(models.Model):
    value = models.CharField(max_length=10, unique=True)

    def __str__(self):
        return f"{self.value:>10}"


class Testresult(models.Model):
    testcase = models.ForeignKey(Testcase, related_name='results', on_delete=models.CASCADE)  # many-to-one
    verdict = models.ForeignKey(Verdict, on_delete=models.CASCADE)  # many-to-one

    def __str__(self):
        return f"{self.testcase}: {self.verdict}"

Take One: use base ModelSerializers and let the ViewSet handle create()

views.py:

from rest_framework import viewsets, status
from rest_framework.response import Response

from .models import Testresult, Testcase, Verdict
from .serializers import StandardTestresultSerializer

class CustomTestresultViewSet(viewsets.ModelViewSet):
    queryset = Testresult.objects.all()
    serializer_class = StandardTestresultSerializer

    def create(self, request, *args, **kwargs):
        try:
            verdict = Verdict.objects.get(value=request.data["verdict.value"])
        except (Verdict.DoesNotExist, KeyError):
            return Response(status=status.HTTP_400_BAD_REQUEST)

        testcase = self._create_or_update_testcase(request)

        testresult = Testresult.objects.create(verdict=verdict, testcase=testcase)
        return Response(StandardTestresultSerializer(testresult).data)

    def _create_or_update_testcase(self, request):
        uid = request.data.get("testcase.uid")
        name = request.data.get("testcase.name")
        try:
            testcase = Testcase.objects.get(uid=uid)
            if testcase.name != name:
                testcase.name = name
                testcase.save()
        except Testcase.DoesNotExist:
            testcase = Testcase.objects.create(**request.data["testcase"])
        return testcase

serializers.py:

from rest_framework import serializers

from .models import Testresult, Verdict, Testcase


class StandardVerdictSerializer(serializers.ModelSerializer):
    class Meta:
        model = Verdict
        fields = ('value',)


class StandardTestcaseSerializer(serializers.ModelSerializer):
    class Meta:
        model = Testcase
        fields = ('uid', 'name')


class StandardTestresultSerializer(serializers.ModelSerializer):
    testcase = StandardTestcaseSerializer()
    verdict = StandardVerdictSerializer()

    class Meta:
        model = Testresult
        fields = ('testcase', 'verdict')

What I like about this approach

The logic for looking up / creating / updating Verdict and Testcase instances and returning error statiuses lives in the View class. From what I've understood so far, serializers are not really intended to use as a way to retrieve already existing instances.

What I don't like

Input validation which for Verdict and Testcase is bypassed, what would normally be done by the serializers. This is currently not too much of a problem, but if (for example) the Verdict model gets a new field date this could become more important.


Take Two: Using a simple View class and more complex Serializers classes

views.py:

class StandardTestresultViewSet(viewsets.ModelViewSet):
    queryset = Testresult.objects.all()
    serializer_class = UnvalidatedTestresultSerializer

serializers.py:

class UnvalidatedVerdictSerializer(serializers.ModelSerializer):
    class Meta:
        model = Verdict
        fields = ('value',)
        extra_kwargs = {
            'value': {'validators': []}
        }

    def create(self, validated_data):
        try:
            return Verdict.objects.get(**validated_data)
        except ObjectDoesNotExist:
            raise serializers.ValidationError(detail="Unknown result value", code=status.HTTP_400_BAD_REQUEST)


class UnvalidatedTestcaseSerializer(serializers.ModelSerializer):
    class Meta:
        model = Testcase
        fields = ('uid', 'name')
        extra_kwargs = {
            'uid': {'validators': []}
        }

    def create(self, validated_data):
        uid = validated_data.get('uid')
        name = validated_data.get('name')
        try:
            testcase = Testcase.objects.get(uid=uid)
            if testcase.name != name:
                testcase.name = name
                testcase.save()
        except ObjectDoesNotExist:
            testcase = Testcase.objects.create(**validated_data)
        return testcase


class UnvalidatedTestresultSerializer(serializers.ModelSerializer):
    testcase = UnvalidatedTestcaseSerializer()
    verdict = UnvalidatedVerdictSerializer()

    class Meta:
        model = Testresult
        fields = ('testcase', 'verdict')

    def create(self, validated_data):
        verdict_data = validated_data.pop('verdict')
        serialized_verdict = UnvalidatedVerdictSerializer(data=verdict_data)
        serialized_verdict.is_valid(raise_exception=True)
        verdict_instance = serialized_verdict.save()

        testcase_data = validated_data.pop('testcase')
        serialized_testcase = UnvalidatedTestcaseSerializer(data=testcase_data)
        serialized_testcase.is_valid(raise_exception=True)
        testcase_instance = serialized_testcase.save()

        return Testresult.objects.create(testcase=testcase_instance, verdict=verdict_instance)

What I like

Validation mechanisms of ModelSerializer can be utilized.

What I don't like

create methods don't actually create Model instances but merely retrieve and udpate them if necessary.
GET requests will also be using the "Unvalidated" serializers, which I don't see as a problem right now, but maybe that is also not a good practice?


Take Three: combining both approaches and select the serializer class to use depending on the request method

In this approach the same serializer classes as shown in Take One and Take Two will be used, so I'm only giving the code for the View class.

views.py:

class DynamicTestresultViewSet(viewsets.ModelViewSet):
    queryset = Testresult.objects.all()

    def get_serializer_class(self):
        if self.request.method == "POST":
            return UnvalidatedTestresultSerializer
        else:
            return StandardTestresultSerializer

What I like

Serializers are used to parse and validate the input data of the request. GET requests will be served using the "normal" serializer.

What I don't like

Requires six serializer classes instead of three.


What approach would you suggest, or would you do it entirely different?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.