Django objects uniqueness hell with M2M fields

user_2255491 0
Viewed 886
class Badge(SafeDeleteModel):
    owner = models.ForeignKey(settings.AUTH_USER_MODEL,
                              blank=True, null=True,
                              on_delete=models.PROTECT)
    restaurants = models.ManyToManyField(Restaurant)
    identifier = models.CharField(max_length=2048)  # not unique at a DB level!

I want to ensure that for any badge, for a given restaurant, it must have a unique identifier. Here are the 4 ideas I have had:

  • idea #1: using unique_together -> Does not work with M2M fields as explained [in documentation] (https://docs.djangoproject.com/en/2.1/ref/models/options/#unique-together)
  • idea #2: overriding save() method. Does not fully work with M2M, because when calling add or remove method, save() is not called.

  • idea #3: using an explicite through model, but since I'm live in production, I'd like to avoid taking risks on migrating important structures like theses. EDIT: after thinking of it, I don't see how it could help actually.

  • idea #4: Using a m2m_changedsignal to check the uniqueness anytime the add() method is called.

I ended up with the idea 4 and thought everything was OK, with this signal...

@receiver(m2m_changed, sender=Badge.restaurants.through)
def check_uniqueness(sender, **kwargs):
    badge = kwargs.get('instance', None)
    action = kwargs.get('action', None)
    restaurant_pks = kwargs.get('pk_set', None)

    if action == 'pre_add':
        for restaurant_pk in restaurant_pks:
            if Badge.objects.filter(identifier=badge.identifier).filter(restaurants=restaurant_pk):
                raise BadgeNotUnique(MSG_BADGE_NOT_UNIQUE.format(
                    identifier=badge.identifier,
                    restaurant=Restaurant.objects.get(pk=restaurant_pk)
                ))

...until today when I found in my database lots of badges with the same identifier but no restaurant (should not happend at the business level) I understood there is no atomicity between the save() and the signal. Which means, if the user have an error about uniqueness when trying to create a badge, the badge is created but without restaurants linked to it.

So, the question is: how do you ensure at the model level that if the signal raises an Error, the save() is not commited?

Thanks!

django-signals django-models django
3 Answers

I see two separate issues here:

  1. You want to enforce a particular constraint on your data.

  2. If the constraint is violated, you want to revert previous operations. In particular, you want to revert the creation of the Badge instance if any Restaurants are added in the same request that violate the constraint.

Regarding 1, your constraint is complicated because it involves multiple tables. That rules out database constraints (well, you could probably do it with a trigger) or simple model-level validation.

Your code above is apparently effective at preventing adds that violate the constraint. Note, though, that this constraint could also be violated if the identifier of an existing Badge is changed. Presumably you want to prevent that as well? If so, you need to add similar validation to Badge (e.g. in Badge.clean()).

Regarding 2, if you want the creation of the Badge instance to be reverted when the constraint is violated, you need to make sure the operations are wrapped in a database transaction. You haven't told us about the views where these objects area created (custom views? Django admin?) so it's hard to give specific advice. Essentially, you want to have this:

with transaction.atomic():
    badge_instance.save()
    badge_instance.add(...)

If you do, an exception thrown by your M2M pre_add signal will rollback the transaction, and you won't get the leftover Badge in your database. Note that admin views are run in a transaction by default, so this should already be happening if you're using the admin.

Another approach is to do the validation before the Badge object is created. See, for example, this answer about using ModelForm validation in the Django admin.

user_23957960

You can specify your own connecting model for your M2M-models, and then add a unique_together constraint in the meta class of the membership model

class Badge(SafeDeleteModel):
    ...
    restaurants = models.ManyToManyField(Restaurant, through='BadgeMembership')

class BadgeMembership(models.Model):
    restaurant = models.ForeignKey(Restaurant, null=False, blank=False, on_delete=models.CASCADE)
    badge = models.ForeignKey(Badge, null=False, blank=False, on_delete=models.CASCADE)

    class Meta:
        unique_together = (("restaurant", "badge"),)

This creates an object that's between the Badge and Restaurant which will be unique for each badge per restaurant.

Badge and restaurant membership

Optional: Save check

You can also add a custom save function where you can manually check for uniqueness. In this way you can manually raise an exception.

class BadgeMembership(models.Model):
    restaurant = models.ForeignKey(Restaurant, null=False, blank=False, on_delete=models.CASCADE)
    badge = models.ForeignKey(Badge, null=False, blank=False, on_delete=models.CASCADE)

    def save(self, *args, **kwargs):
        # Only save if the object is new, updating won't do anything
        if self.pk is None:
            membershipCount = BadgeMembership.objects.filter(
                Q(restaurant=self.restaurant) &
                Q(badge=self.badge)
            ).count()
            if membershipCount > 0:
                raise BadgeNotUnique(...);
            super(BadgeMembership, self).save(*args, **kwargs)
user_104000500

I'm afraid the correct way to achieve this really is by adapting the "through" model. But remember that at database level this "through" model already exists, and therefore your migration would simply be adding a unique constraint. It's a rather simple operation, and it doesn't really involve any real migrations, we do it often in production environments.

Take a look at this example, it pretty much sums everything you need.

user_11282230
Related