Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#14731 closed (fixed)

[Patch] Change 14413 breaks old fixtures with permissions

Reported by: Christian Hammond Owned by: nobody
Component: contrib.auth Version: 1.3-alpha
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Changeset 14413 introduced a regression with fixtures containing permissions. Any old fixtures that contain data for permissions on an object (such as a User) will fail on any unit test using them, or during a loaddata.

The cause appears to be that when creating the permissions in contrib/auth/management/, the ordering used before this change is no longer maintained, and as such the IDs of the permissions no longer match up. When a fixture attempts to install the permission, it will attempt to use a new ID that doesn't match what Django is now creating.

Updating the fixtures to use the new IDs results in regressions on older versions of Django for the same reason. This makes maintaining installable software that attempts to provide compatibility between Django versions a very difficult task.

I understand that the new logic is intended to speed up the test suite, but it's a fairly major regression.

To reproduce this, simply create a new site using an older Django revision (say, 14412), run syncdb, create the admin user, and then dumpdata to a fixture. Create a dummy unit test that loads the fixture. Run it and it will succeed. Upgrade to revision 14413 or newer and re-run the unit test. It will fail.

On sqlite, the error message is:

Problem installing fixture '/home/chipx86/temp/django-breakage-test/fixturetest/../fixturetest/myapp/fixtures/test_users.json': Traceback (most recent call last):
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/commands/", line 174, in handle
  File "/usr/local/lib/python2.6/dist-packages/django/core/serializers/", line 165, in save
    models.Model.save_base(self.object, using=using, raw=True)
  File "/usr/local/lib/python2.6/dist-packages/django/db/models/", line 518, in save_base
    rows = manager.using(using).filter(pk=pk_val)._update(values)
  File "/usr/local/lib/python2.6/dist-packages/django/db/models/", line 490, in _update
    return query.get_compiler(self.db).execute_sql(None)
  File "/usr/local/lib/python2.6/dist-packages/django/db/models/sql/", line 864, in execute_sql
    cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
  File "/usr/local/lib/python2.6/dist-packages/django/db/models/sql/", line 730, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python2.6/dist-packages/django/db/backends/sqlite3/", line 221, in execute
    return Database.Cursor.execute(self, query, params)
IntegrityError: columns content_type_id, codename are not unique

The ordering appears to be incorrect due to the use of a set() for searched_perms. Switching to using a list for this solves the problem.

Providing a patch for this.

Attachments (2)

permission-id-fix-20101119.diff (914 bytes) - added by Christian Hammond 6 years ago.
Patch to retain a consistent order for permission IDs.
permission-id-fix-20101122.diff (10.8 KB) - added by Christian Hammond 6 years ago.
Patch for permission ordering with regression tests

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by Christian Hammond

Patch to retain a consistent order for permission IDs.

comment:1 Changed 6 years ago by Karen Tracey

milestone: 1.3
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Since this is a reported regression, it must be resolved before 1.3.

comment:2 Changed 6 years ago by Alex Gaynor

Needs tests: set
Triage Stage: UnreviewedAccepted

Changing that to a list shouldn't affect anything, from a performance perspective (might even speed stuff up.... by a miniscule margin). I'd like a test though, its subtly enough that it wouldn't surprise me if it was reintroduced ;)

Changed 6 years ago by Christian Hammond

Patch for permission ordering with regression tests

comment:3 Changed 6 years ago by Christian Hammond

The new patch includes regression tests. Let me know if you want anything done differently there.

The regression test has been tested with r14412 (last known good revision), r14412 (first broken revision), and r14678 (HEAD at the time of this writing), both with and without the fix.

comment:4 Changed 6 years ago by Jannis Leidel

Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:5 Changed 6 years ago by Christian Hammond

Hi. Just wondering if there's any ETA on this. It's kind of blocking me.

comment:6 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

(In [14891]) Fixed #14731 -- Respect ordering when creating the default permissions. Thanks, chipx86.

comment:7 Changed 6 years ago by Christian Hammond

Thanks! :)

comment:8 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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