Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can I optimize my query when using assign_perm? #709

Closed
agusawa opened this issue Aug 20, 2020 · 2 comments
Closed

How can I optimize my query when using assign_perm? #709

agusawa opened this issue Aug 20, 2020 · 2 comments

Comments

@agusawa
Copy link

agusawa commented Aug 20, 2020

I'm currently working on a project where one room has its own group permissions.

When someone creates a new room, the system will do the following:

  1. Creating a room
  2. Create owner and staff groups with unique names, such as Room Owner#room#{pk}
  3. Get list of owner and staff permissions from Room class
  4. Make an assignment to each group
  5. Assign owner group permissions to the user

Here is my Room model :

class Room(models.Model):
	# ...fields

	PERMISSION_GROUPS = [
		'owner': [
			('view_room', _('...')),
			('change_room', _('...')),
			('delete_room', _('...')),

			('open_room', _('...')),
			('close_room', _('...'))
		],
		'staff': [
			('view_room', _('...')),
			('open_room', _('...')),
			('close_room', _('...'))	
		]
	]

This is my code for create room and generate group permissions:

from guardian.shortcuts import assign_perm

class CreateRoomView(generics.CreateAPIView):
    def _get_permission_string(self, perm):
        """
        Get string permission, the return is like 'room.add_room'
        """
        return f'room.{perm.codename}'

    def post(self):
        # Create a room
        body = {}
        room = Room.objects.create(**body)
        
        # Get content type
        ctype = ContentType.objects.get_for_model(Room)
        
        # Define name of owner and staff group for this room permission
        owner_group_name = _('Room Owner#room#{pk}')
        staff_group_name = _('Room Staff#room#{pk}')
        
        # Create group
        owner_group = Group.objects.create(
        	name=owner_group_name.format(pk=room.pk))
        staff_group = Group.objects.create(
        	name=staff_group_name.format(pk=room.pk))
        
        # Get permissions by group permissions
        owner_perms = Permission.objects.filter(
            codename__in=Room.PERMISSION_GROUPS.get('owner'), content_type=ctype)
        staff_perms = Permission.objects.filter(
            codename__in=Room.PERMISSION_GROUPS.get('staff'), content_type=ctype)
        
        # Assign owner permissions to the group
        for owner_perm in owner_perms:
            perm = self._get_permission_string(owner_perm)
            assign_perm(perm, owner_group, obj=room)
        
        # Assign staff permissions to the group
        for staff_perm in staff_perms:
            perm = self._get_permission_string(staff_perm)
            assign_perm(perm, staff_group, obj=room)
        
        # Assign owner group permission to the user
        request.user.groups.add(owner_group)

When I look at the query logs, there are a lot of queries being performed in this process.

Actually there are still a few groups and the assignment process as above, but I only included a few examples, so there are not too many.If calculated, there may be more than 50 query executions that run in this process.

So, how can I optimize the query for this problem? Can anyone share how to do it better.

Thank you,

@michael-k
Copy link
Collaborator

  1. Use direct foreign keys, see https://django-guardian.readthedocs.io/en/stable/userguide/performance.html#direct-foreign-keys

  2. You can pass instances of Permission to assign_perm. perm = self._get_permission_string(owner_perm) leads to an additional query per permission.

  3. If this isn't enough you can assign the permissions in bulk, see code below. But be aware that I didn't test this code. It might need adjustments, it might not work at all.


Before:

# Assign owner permissions to the group
for owner_perm in owner_perms:
    perm = self._get_permission_string(owner_perm)
    assign_perm(perm, owner_group, obj=room)
        
# Assign staff permissions to the group
for staff_perm in staff_perms:
    perm = self._get_permission_string(staff_perm)
    assign_perm(perm, staff_group, obj=room)

After:

# RoomGroupObjectPermission is the model from suggestion 1, use direct foreign keys
room_group_object_permissions = [
    RoomGroupObjectPermission(
        permission=owner_perm, group=owner_group, content_object=room,
    )
    for owner_perm in owner_perms
] + [
    RoomGroupObjectPermission(
        permission=staff_perm, group=staff_group, content_object=room,
    )
    for staff_perm in staff_perms
]

RoomGroupObjectPermission.objects.bulk_create(room_group_object_permissions)

@agusawa
Copy link
Author

agusawa commented Aug 21, 2020

Omg. I missed this one. I decided to use the first one. Thank you,

@agusawa agusawa closed this as completed Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants