Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#35488 closed Bug (fixed)

BaseModelFormSet.validate_unique() raises unhashable type error for unique fields with unhashable types

Reported by: Hanne Moa Owned by: Madalin Popa
Component: Forms Version: 5.0
Severity: Normal Keywords: JSONField, unique, formset, json, hashable
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Given a User based on AbstractUser and a model:

class DestinationConfig(model.Model):
    class Meta:
        constraints = [models.UniqueConstraint(fields=["user", "settings"], name="unique_destination_per_user")]

    user = models.ForeignKey(User, on_delete=models.CASCADE)
    settings = models.JSONField()

Admin like so:

from django.contrib import admin
from django.contrib.auth.admin import UserAdmin as BaseUserAdmin

class DestinationConfigInline(admin.TabularInline):
    model = DestinationConfig
    ordering = ["media", "label"]
    extra = 1

    def get_formset(self, request, obj=None, **kwargs):
        self.parent_obj = obj
        return super(DestinationConfigInline, self).get_formset(request, obj, **kwargs)

    def get_queryset(self, request):
        qs = super(DestinationConfigInline, self).get_queryset(request)
        return qs.filter(user=self.parent_obj)

class UserAdmin(BaseUserAdmin):
    inlines = [DestinationConfigInline]

admin.site.register(User, UserAdmin)

Then attempting to save a change to a specific user leads to a traceback like this:

Environment:


Request Method: POST
Request URL: https://ACENSORED.DOMAIN/admin/APP_auth/user/65/change/

Django Version: 5.0.6
Python Version: 3.10.12
Installed Applications:
['channels',
 'django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'corsheaders',
 'social_django',
 'rest_framework',
 'rest_framework.authtoken',
 'drf_spectacular',
 'django_filters',
 'phonenumber_field',
 'argus.auth',
 'argus.incident',
 'argus.ws',
 'argus.notificationprofile',
 'argus.dev']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'corsheaders.middleware.CorsMiddleware',
 'whitenoise.middleware.WhiteNoiseMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'social_django.middleware.SocialAuthExceptionMiddleware',
 'django.contrib.auth.middleware.RemoteUserMiddleware']

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.10/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 688, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 134, in _wrapper_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/views/decorators/cache.py", line 62, in _wrapper_view_func
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/sites.py", line 242, in inner
    return view(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1889, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 46, in _wrapper
    return bound_method(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 134, in _wrapper_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1747, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1797, in _changeform_view
    if all_valid(formsets) and form_validated:
  File "/usr/local/lib/python3.10/site-packages/django/forms/formsets.py", line 579, in all_valid
    return all([formset.is_valid() for formset in formsets])
  File "/usr/local/lib/python3.10/site-packages/django/forms/formsets.py", line 579, in <listcomp>
    return all([formset.is_valid() for formset in formsets])
  File "/usr/local/lib/python3.10/site-packages/django/forms/formsets.py", line 384, in is_valid
    self.errors
  File "/usr/local/lib/python3.10/site-packages/django/forms/formsets.py", line 366, in errors
    self.full_clean()
  File "/usr/local/lib/python3.10/site-packages/django/forms/formsets.py", line 456, in full_clean
    self.clean()
  File "/usr/local/lib/python3.10/site-packages/django/forms/models.py", line 789, in clean
    self.validate_unique()
  File "/usr/local/lib/python3.10/site-packages/django/forms/models.py", line 831, in validate_unique
    if row_data in seen_data:

Exception Type: TypeError at /admin/APP_auth/user/65/change/
Exception Value: unhashable type: 'dict'
  • row_data was ('user', {'email_address': 'CENSORED@ANOTHERCENSORED.DOMAIN', 'html': True})
  • seen_data was set().

Is it a design decision that JSONFields cannot be unique/in UniqueConstraints?

  • If yes, can it be documented, preferably with a note as to what to do if you *do* want a unique JSONField?
  • If no, can validate_unique be changed to work in this instance?

This has also been tested on:

  • Django Version: 4.2.11
  • Python Version: 3.10.14

Change History (12)

comment:1 by Sarah Boyce, 4 months ago

Component: UncategorizedForms
Summary: Unexpected traceback: A JSONField being in a UniqueConstraint is handled/documented poorlyBaseModelFormSet.validate_unique() raises TypeError unhashable type: 'dict' for JSONFields in a UniqueConstraint
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

This is different to #23964 but could be fixed as part of #23964.

Is it a design decision that JSONFields cannot be unique/in UniqueConstraints?

No, looks like you have uncovered a bug specific to model formsets 👍 thank you for the report

comment:2 by Simon Charette, 4 months ago

Keywords: formset hashable added; UniqueConstraint removed

The problem happens to manifests itself for JSONField with UniqueConstraint but it happens for any field with a non-hashable value that has a unique constraint defined on it.

e.g.

class FooBar(models.Model):
    settings = models.JSONField(unique=True)

Would exhibit the same problem and the same could be said of HStoreField or other custom fields dealing with dict, set, and other non-hashable data types.

Using django.utils.make_hashable in BaseModelFormSet.validate_unique seems like a potential low-lift solution but even this function can raise a TypeError if dealing with non-hashable value so maybe we're better off silencing these TypeError and let the model level unique constraint validation kick in.

I think the latter would be a better approach because uniqueness on such fields cannot be determined at the Python level. For example, JSONField relies on the jsonb type on Postgres so the ordering of keys is not preserved which means that {"foo": "bar", "bar": "foo"} would be considered equal to {"bar": "foo", "foo": "bar"} but not on MySQL which json type is a basically longtext with JSON validation and preserves insertion order.

comment:3 by Simon Charette, 4 months ago

I think this has little to do with #23964 (which focuses on Meta.constraints) and is more of an analogous to #26819 (which was for another field using non-hashable types) but for JSONField.

  • django/forms/models.py

    diff --git a/django/forms/models.py b/django/forms/models.py
    index 4cda4e534e..42feeac5c2 100644
    a b  
    2323    SelectMultiple,
    2424)
    2525from django.utils.choices import BaseChoiceIterator
     26from django.utils.hashable import make_hashable
    2627from django.utils.text import capfirst, get_text_list
    2728from django.utils.translation import gettext
    2829from django.utils.translation import gettext_lazy as _
    def validate_unique(self):  
    835836                        d._get_pk_val()
    836837                        if hasattr(d, "_get_pk_val")
    837838                        # Prevent "unhashable type: list" errors later on.
    838                         else tuple(d) if isinstance(d, list) else d
     839                        else make_hashable(d)
    839840                    )
    840841                    for d in row_data
    841842                )

The problem IMO is that 06a11ef6ecf324db0a1530b8cca727883698f442 focused on one type on unhashable value instead of the generic problem.

Last edited 4 months ago by Simon Charette (previous) (diff)

comment:4 by Sarah Boyce, 4 months ago

Summary: BaseModelFormSet.validate_unique() raises TypeError unhashable type: 'dict' for JSONFields in a UniqueConstraintBaseModelFormSet.validate_unique() raises unhashable type error for unique fields with unhashable types

Yes I agree, thank you Simon 👍

comment:5 by Simon Charette, 4 months ago

Easy pickings: set

Marking as easy picking as all that is required here is likely to submit a PR with the change above to use make_hashable and add a test similar to the one added in 06a11ef6ecf324db0a1530b8cca727883698f442 but using a form.JSONField instead.

It might be a good one to pick up if you've never contributed to Django Hanne!

comment:6 by DraKen0009, 4 months ago

Owner: changed from nobody to DraKen0009
Status: newassigned

comment:7 by DraKen0009, 4 months ago

Owner: DraKen0009 removed
Status: assignednew

comment:8 by Madalin Popa, 4 months ago

Owner: set to Madalin Popa
Status: newassigned

comment:9 by Madalin Popa, 4 months ago

Has patch: set

comment:10 by Sarah Boyce, 4 months ago

Triage Stage: AcceptedReady for checkin

comment:11 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

Resolution: fixed
Status: assignedclosed

In d28626e:

Fixed #35488 -- Fixed BaseModelFormSet.validate_unique() crash due to unhashable type.

comment:12 by Sarah Boyce <42296566+sarahboyce@…>, 4 months ago

In dbd1a8bd:

[5.1.x] Fixed #35488 -- Fixed BaseModelFormSet.validate_unique() crash due to unhashable type.

Backport of d28626ecf8bd340084ed70ff2d88e8dbab001e2c from main.

Note: See TracTickets for help on using tickets.
Back to Top