Opened 4 years ago

Closed 2 years ago

#16509 closed Bug (needsinfo)

init test db: try insert a existing permission (django.db.utils.IntegrityError: columns content_type_id, codename are not unique)

Reported by: rogeliomita@… Owned by: tobias
Component: contrib.contenttypes Version: 1.4
Severity: Release blocker Keywords: not unique, permission, auth
Cc: patrick.craston@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a problem with django 1.3, south 0.7.3, not which of them ocacion the problem, but I tell that happens to me:
debuging I see that the problem is that in a migration try add the permission 'Can add profile' again and the table constraint auth_permission does not allow it and raise.
Viewing source for south found nothing, and seeing the source of django I found this fragment in which I found a problem, but I not know his reason

-django/contrib/auth/management/init.py

def create_permissions(app, created_models, verbosity, **kwargs):
    from django.contrib.contenttypes.models import ContentType

    app_models = get_models(app)

    # This will hold the permissions we're looking for as
    # (content_type, (codename, name))
    searched_perms = list()
    # The codenames and ctypes that should exist.
    ctypes = set()
    for klass in app_models:
        ctype = ContentType.objects.get_for_model(klass)
        ctypes.add(ctype)
        for perm in _get_all_permissions(klass._meta):
            searched_perms.append((ctype, perm))

    # Find all the Permissions that have a context_type for a model we're
    # looking for.  We don't need to check for codenames since we already have
    # a list of the ones we're going to create.
    all_perms = set(auth_app.Permission.objects.filter(
        content_type__in=ctypes,
    ).values_list(
        "content_type", "codename"
    ))

    for ctype, (codename, name) in searched_perms:
        # If the permissions exists, move on.
        if (ctype.pk, codename) in all_perms:
            continue
        p = auth_app.Permission.objects.create(
            codename=codename,
            name=name,
            content_type=ctype
        )
        if verbosity >= 2:
            print "Adding permission '%s'" % p

Looking at the result of searched_perms variable, I found repeated permissions

searched_perms: [(<ContentType: profile>, (u'add_profile', u'Can add profile')), (<ContentType: profile>, (u'change_profile', u'Can change profile')), (<ContentType: profile>, (u'delete_profile', u'Can delete profile')), (<ContentType: profile>, ('add_profile', 'Can add profile')), (<ContentType: profile>, ('change_profile', 'Can change profile')), (<ContentType: profile>, ('delete_profile', 'Can delete profile')), (<ContentType: profile>, ('view_profile', 'Can view profile'))].

Converting searched_perms to a set and solve the problem by eliminating repetitions, but completely ignores the cause. (sorry for my bad English)

for ctype, (codename, name) in searched_perms: -------> for ctype, (codename, name) in set(searched_perms):

Change History (14)

comment:1 Changed 4 years ago by rogeliomita@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

in saw in https://code.djangoproject.com/changeset/14891/django/trunk/django/contrib/auth/management/__init__.py conversion

searched_perms = set() 
searched_perms = list()
searched_perms.add((ctype, perm)) 
searched_perms.append((ctype, perm))

but it seems that the order matters, then a set is not a solution.

comment:2 Changed 4 years ago by aaugustin

  • Component changed from contrib.auth to contrib.contenttypes
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

This looks like it's a regression introduced by r14891.

comment:3 Changed 4 years ago by russellm

  • Severity changed from Normal to Release blocker

Regressions are automatically release blockers.

comment:4 Changed 3 years ago by claudep

To be able to reproduce the error (and create a test), the question is: how did you manage to have a contenttype (profile) twice in the same app?

comment:5 Changed 3 years ago by anonymous

it seems to me that the problem is initiated by south. That's why I did not know who is responsible for determining the location for this ticket. I could apply the migration changing that part of the code. Sorry I do not remember very well how to play the event, I will try to find the problem again to verify.

comment:6 Changed 3 years ago by roger

I checked again and the error is caused by a migration what south does not interpret in a single run (run it again after, and it works), I suppose, must be a bad dependencies that a previous migration. Sorry about the waste of time.

comment:7 Changed 3 years ago by tobias

  • Owner changed from nobody to tobias
  • Status changed from new to assigned

comment:8 Changed 3 years ago by tobias

It is true that this is a regression insofar as searched_perms used to be a set, but I am not sure how to reproduce this in a test. It also sounds like the original ticket author has since discovered that this was an issue with the South migration and was able to fix it outside of Django.

If needed, the fix would be easy enough:

for perm in _get_all_permissions(klass._meta):
    if (ctype, perm) not in searched_perms:
        searched_perms.append((ctype, perm))

instead of:

for perm in _get_all_permissions(klass._meta):
    searched_perms.append((ctype, perm))

comment:9 Changed 3 years ago by tobias

  • Resolution set to worksforme
  • Status changed from assigned to closed

claudep and I were unable to reproduce this issue

comment:10 follow-up: Changed 3 years ago by patrick

  • Cc patrick.craston@… added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Reopening this ticket as I am experiencing the same problem.

After updating from Django 1.2.7 to 1.3.1 and running our Unit Test suite with sqlite, the unit tests fail with the IntegrityError mentioned in this ticket.

I was able to fix the problem by moving this code block (lines 38-42, source)

all_perms = set(auth_app.Permission.objects.filter(
            content_type__in=ctypes,
            ).values_list(
            "content_type", "codename"
            ))

from outside the loop (line 44, source)

for ctype, (codename, name) in searched_perms:

to inside this for loop.

This is the new code:

for ctype, (codename, name) in searched_perms:
        # Find all the Permissions that have a context_type for a model we're
        # looking for.  We don't need to check for codenames since we already have
        # a list of the ones we're going to create.
        ## Bug fix ##
        # Moved the following code block to inside the for loop due to error.
        # Problem was that permission was being set twice for apps specific to project. If the code is outside
        # the loop it will only check once per set of apps. 
        all_perms = set(auth_app.Permission.objects.filter(
            content_type__in=ctypes,
            ).values_list(
            "content_type", "codename"
            ))
        
        # If the permissions exists, move on.
        if (ctype.pk, codename) in all_perms:
            continue
        p = auth_app.Permission.objects.create(
            codename=codename,
            name=name,
            content_type=ctype
        )
        if verbosity >= 2:
            print "Adding permission '%s'" % p

It seems like this code has changed in the Django subversion trunk, so the bug might have been fixed there.

comment:11 in reply to: ↑ 10 Changed 3 years ago by aaugustin

Replying to patrick:

It seems like this code has changed in the Django subversion trunk, so the bug might have been fixed there.

This is probably fixed in trunk, since two people failed to reproduce the issue on trunk two months ago. Please reopen the ticket again if you can confirm that the issue still exists on trunk.

comment:12 Changed 3 years ago by aaugustin

  • Resolution set to worksforme
  • Status changed from reopened to closed

comment:13 Changed 3 years ago by patrick

  • Resolution worksforme deleted
  • Status changed from closed to reopened
  • Version changed from 1.3 to 1.4

I have just migrated my project to Django 1.4 and the problem reported by me above still persists so I am reopening this ticket.

comment:14 Changed 2 years ago by akaariai

  • Resolution set to needsinfo
  • Status changed from reopened to closed

Without a test case (or zipped test project as attachment) we can't do anything here.

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