Skip unnecessary unique together checking by yuekui · Pull Request #9154 · encode/django-rest-framework (original) (raw)

Upgrading from 3.14.0 to 3.15.1 caused tests for a project of mine to break, and I think it's related to or because of this PR.

However, I don't know that I would consider that a regression for DRF, and I was able to find a workaround (see below).

Example Code

I apologize in advance, I can only post redacted example code.

Model:

class RedactedModel(models.Model): ... fk = models.ForeignKey(...) status = models.CharField(..., default="not-primary-1") usage = models.CharField(...)

class Meta:
    constraints = [
        models.UniqueConstraint(
            name="usage_unique_status_idx",
            fields=["fk", "usage"],
            condition=models.Q(status="primary"),
        ),
    ]

Serializer:

class RedactedModelSerializer(serializers.HyperlinkedModelSerializer): ... fk = serializers.HyperlinkedRelatedField( queryset=FKModel.objects.all(), ..., ) status = serializers.ReadOnlyField(default="not-primary-1")

class Meta:
    model = RedactedModel
    fields = ['fk', 'status', 'usage', ...]

ViewSet:

class RedactedModelViewSet(viewsets.ModelViewSet): queryset = RedactedModel.objects.all() serializer_class = RedactedModelSerializer ...

def perform_create(self, serializer, *args, **kwargs):
    with transaction.atomic():
        try:
            RedactedModel.objects.filter(
                fk=serializer.validated_data['fk'],
                status="primary",
                usage=serializer.validated_data['usage'],
            ).update(status="not-primary-2")
        except IntegrityError:
            ...
        serializer.save()

I'm not 100% sure that I am doing everything correctly, so if there's something obviously amiss, or even a better way to implement this, please let me know.

The Problem

For quick reference, here's some relevant DRF code:

class CreateModelMixin: def create(self, request, *args, **kwargs): serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) self.perform_create(serializer) headers = self.get_success_headers(serializer.data) return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

Originally (eg: with DRF 3.14.0), serializer validation seems to have skipped checking the unique together fields, which allowed my custom perform_create method to update the previous primary RedactedModel object before saving the new object, avoiding violating the usage_unique_status_idx unique together constraint.

After updating to 3.15.1, the serializer validation no longer skips checking the unique together fields, so the serializer never validates the data, previous RedactedModel objects are never updated, and a new RedactedModel is not created.

The Solution

I found a clue in ModelSerializer.get_validators:

class ModelSerializer(Serializer): def get_validators(self): """ Determine the set of validators to use when instantiating serializer. """ # If the validators have been declared explicitly then use that. validators = getattr(getattr(self, 'Meta', None), 'validators', None) if validators is not None: return list(validators)

    # Otherwise use the default set of validators.
    return (
        self.get_unique_together_validators() +
        self.get_unique_for_date_validators()
    )

It's a bit of an imperfect workaround, but I set the validators to an empty list in the serializer Meta class:

class RedactedModelSerializer(serializers.HyperlinkedModelSerializer): ...

class Meta:
    model = RedactedModel
    # Turn off unique together validators so we can validate serialized data
    # and handle non-unique situations manually during creation
    validators = []
    ...

This bypasses serializer validation for the unique constraints so the update code in perform_create have its affect.

I'm not sure that this workaround is reasonable for other use cases, but I'm also not sure if this is uncovering a bug in a previous version (3.14.0), if it is uncovering a regression somewhere between 3.14.0 and 3.15.1, or if it is uncovering a bug in my codebase. But I hope this might help somebody else who runs into this.