Code

Opened 5 years ago

Last modified 2 months ago

#10827 new Bug

django.auth create_permissions must clear the content type cache before creating permissions

Reported by: seanl Owned by: nobody
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: jodym@…, chris+django@…, rjalves@…, jorgecarleitao, julenx@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I hit a problem which took some time to track down, where at the DB flush stage in a sequence of tests (using TransactionTestCase) the recreation of permissions was failing with a FK constraint error.

This was caused by inserting a permission referring to a content type that didn't exist in the DB.

This happened because the content type was still in the cache, even thought the django_content_type table had been truncated.

The cache hadn't been cleared because post_syncdb signal dispatch had called create_permissions before calling update_contenttypes (which does clear the cache and recreate the content types correctly).

The real problem here is that create_permissions and update_contenttypes are both connected to the post_syncdb signal, with the former depending on the latter having been run first, but the dispatcher doesn't guarantee order of dispatch (or rather it dispatches in the order the signal handlers are connected, but that order depends on the order in which modules are loading which is not well-defined). Unfortunately that's a hard problem to solve, and I don't have any ideas about that short of substantive changes to the signal dispatcher.

An easier solution to this particular problem is for create_permissions to clear the content types cache before it recreates permissions, that way the necesarry content types will be created as needed (see attached patch).

Attachments (2)

10827-create-permissions-must-clear-content-type-cache.diff (561 bytes) - added by seanl 5 years ago.
contenttype_error_on_TransactionTestCase.tar.bz2 (3.5 KB) - added by rjalves 14 months ago.
Test Django project - created with Django 1.4.5

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by seanl

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by Jason Yan <tailofthesun@…>

I just got hit by this same issue and worked around it in a similar manner. I instead removed the signal that was connected to update_contenttypes and moved the update_contenttypes call to django.core.management.commands.flush.

For anyone that might be possibly searching for this, here's the exception from the tests without the fix:

IntegrityError: insert or update on table "auth_permission" violates foreign key constraint "content_type_id_refs_id_728de91f"
DETAIL:  Key (content_type_id)=(2) is not present in table "django_content_type".

comment:4 Changed 3 years ago by PeterRussell

It seems that the reason that this happens is that the syncdb command is firing the post_syncdb signal for each application, as returned from get_apps() [source:/django/trunk/django/core/management/sql.py@14849#L178 here]. Get_apps appears to return apps in app-name order (see [source:/django/trunk/django/db/models/loading.py@14849#L120 here]), and auth comes before contenttypes asciibetically. It seems the right solution is probably to have a separate post_contenttypes_refreshed signal, and have create_permissions listen for that. It would also be great if updating permissions could fire its own signal, so you can update groups etc based on that. (We've been having a somewhat similar issue).

comment:5 Changed 3 years ago by SmileyChris

  • Severity set to Normal
  • Type set to Bug

comment:6 Changed 3 years ago by julien

  • Needs tests set

comment:7 Changed 3 years ago by orb@…

  • Easy pickings unset
  • UI/UX unset

I'm having this problem too. We worked around it by extending TransactionTestCase so that _pre_setup() invokes ContentType.objects.clear_cache(). I'd really love to see a solution to this.

comment:8 Changed 2 years ago by jodym@…

  • Cc jodym@… added

comment:9 Changed 2 years ago by anonymous

A workaround based on comment 7 is available here: http://djangosnippets.org/snippets/2752/

comment:10 Changed 22 months ago by gcc

+1. This just bit us as well.

I think if flush is going to invalidate caches (which it appears to do), then it should be sending a signal that allows holders of caches to respond appropriately. So something like this in flush.py:

for app in set(all_models):
    models.signals.post_flush.send(sender=app, app=app,
        created_models=created_models, verbosity=verbosity,
        interactive=interactive, db=db)
emit_post_sync_signal(set(all_models), verbosity, interactive, db)

Assuming a new signal flush which ContentTypeManager listens for, and then flushes its cache.

comment:11 Changed 22 months ago by gcc

  • Cc chris+django@… added

Changed 14 months ago by rjalves

Test Django project - created with Django 1.4.5

comment:12 Changed 14 months ago by rjalves

  • Cc rjalves@… added

The attached file is a fresh django project created with Django 1.4.5 which reproduces the mentioned error.
NOTE: This error is not reproducible with the sqlite backend (MySQL was not tested but was reported to fail with InnoDB in other tickets #9207).

Ticket #9207 is closed as fixed but due to lack of reproducibility. From the error it looks like a duplicate of the current.
The workaround described on comment 3 from #9207 also solves the problem (putting contrib.contenttypes before contrib.auth in INSTALLED_APPS).

PS: I don't know if there's any reason why INSTALLED_APPS has the current order but having django-admin.py invert the order could workaround this problem.

Last edited 14 months ago by rjalves (previous) (diff)

comment:13 Changed 14 months ago by rjalves

Another duplicate: #6259

comment:14 Changed 8 months ago by anonymous

I think I have run into the same problem while trying to use Selenium to test Django admin capabilities. When I try to update a user's permissions, I get the FK error. You can see [my code and issue on SO](http://stackoverflow.com/questions/18281137/selenium-django-gives-foreign-key-error). So I think this is still a present bug in Django 1.5.1 / MySQL.

comment:15 Changed 4 months ago by jorgecarleitao

  • Cc jorgecarleitao added

comment:16 Changed 2 months ago by julen

  • Cc julenx@… added

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.