Skip to content

Commit

Permalink
feat: Add cache for permission checks (#1486)
Browse files Browse the repository at this point in the history
  • Loading branch information
fsbraun authored Aug 20, 2024
1 parent 3d29fdd commit 01702d8
Show file tree
Hide file tree
Showing 11 changed files with 312 additions and 3 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ jobs:
run: echo "CUSTOM_IMAGE=custom_image.Image" >> $GITHUB_ENV
if: ${{ matrix.custom-image-model }}
- name: Run coverage
run: coverage run setup.py test
run: coverage run tests/settings.py
- name: Upload Coverage to Codecov
uses: codecov/codecov-action@v1
4 changes: 4 additions & 0 deletions filer/admin/folderadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from easy_thumbnails.models import Thumbnail

from .. import settings
from ..cache import clear_folder_permission_cache
from ..models import File, Folder, FolderPermission, FolderRoot, ImagesWithMissingData, UnsortedImages, tools
from ..settings import FILER_IMAGE_MODEL, FILER_PAGINATE_BY, TABLE_LIST_TYPE
from ..thumbnail_processors import normalize_subject_location
Expand Down Expand Up @@ -107,6 +108,9 @@ def save_form(self, request, form, change):
Given a ModelForm return an unsaved instance. ``change`` is True if
the object is being changed, and False if it's being added.
"""
if not change:
# New folder invalidates the folder permission cache (or it will not be visible)
clear_folder_permission_cache(request.user)
r = form.save(commit=False)
parent_id = request.GET.get('parent_id', None)
if not parent_id:
Expand Down
9 changes: 9 additions & 0 deletions filer/admin/permissionadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.utils.translation import gettext_lazy as _

from .. import settings
from ..cache import clear_folder_permission_cache


class PermissionAdmin(admin.ModelAdmin):
Expand Down Expand Up @@ -31,3 +32,11 @@ def get_model_perms(self, request):
'change': enable_permissions,
'delete': enable_permissions,
}

def save_model(self, request, obj, form, change):
clear_folder_permission_cache(request.user)
super().save_model(request, obj, form, change)

def delete_model(self, request, obj):
clear_folder_permission_cache(request.user)
super().delete_model(request, obj)
84 changes: 84 additions & 0 deletions filer/cache.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import typing

Check failure on line 1 in filer/cache.py

View workflow job for this annotation

GitHub Actions / isort

Imports are incorrectly sorted and/or formatted.

from django.core.cache import cache

from django.contrib.auth import get_user_model


User = get_user_model()


def get_folder_perm_cache_key(user: User, permission: str) -> str:
"""
Generates a unique cache key for a given user and permission.
The key is a string in the format "filer:perm:<permission>", i.e. it does not
contain the user id. This will be sufficient for most use cases.
Patch this method to include the user id in the cache key if necessary, e.g.,
for far more than 1,000 admin users to make the cached unit require less memory.
Parameters:
user (User): The user for whom the cache key is being generated.
permission (str): The permission for which the cache key is being generated.
Returns:
str: The generated cache key.
"""
return f"filer:perm:{permission}"


def get_folder_permission_cache(user: User, permission: str) -> typing.Optional[dict]:
"""
Retrieves the cached folder permissions for a given user and permission.
If the cache value exists, it returns the permissions for the user.
If the cache value does not exist, it returns None.
Parameters:
user (User): The user for whom the permissions are being retrieved.
permission (str): The permission for which the permissions are being retrieved.
Returns:
dict or None: The permissions for the user, or None if no cache value exists.
"""
cache_value = cache.get(get_folder_perm_cache_key(user, permission))
if cache_value:
return cache_value.get(user.pk, None)
return None


def clear_folder_permission_cache(user: User, permission: typing.Optional[str] = None) -> None:
"""
Clears the cached folder permissions for a given user.
If a specific permission is provided, it clears the cache for that permission only.
If no specific permission is provided, it clears the cache for all permissions.
Parameters:
user (User): The user for whom the permissions are being cleared.
permission (str, optional): The specific permission to clear. Defaults to None.
"""
if permission is None:
for perm in ['can_read', 'can_edit', 'can_add_children']:
cache.delete(get_folder_perm_cache_key(user, perm))
else:
cache.delete(get_folder_perm_cache_key(user, permission))


def update_folder_permission_cache(user: User, permission: str, id_list: list[int]) -> None:
"""
Updates the cached folder permissions for a given user and permission.
It first retrieves the current permissions from the cache (or an empty dictionary if none exist).
Then it updates the permissions for the user with the provided list of IDs.
Finally, it sets the updated permissions back into the cache.
Parameters:
user (User): The user for whom the permissions are being updated.
permission (str): The permission to update.
id_list (list): The list of IDs to set as the new permissions.
"""
perms = get_folder_permission_cache(user, permission) or {}
perms[user.pk] = id_list
cache.set(get_folder_perm_cache_key(user, permission), perms)
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
# Generated by Django 3.2.25 on 2024-08-19 14:49

Check failure on line 1 in filer/migrations/0001_squashed_0016_alter_folder_index_together_remove_folder_level_and_more.py

View workflow job for this annotation

GitHub Actions / isort

Imports are incorrectly sorted and/or formatted.

from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion
import filer.fields.multistorage_file
import filer.models.filemodels
import filer.models.mixins


class Migration(migrations.Migration):

replaces = [('filer', '0001_initial'), ('filer', '0002_auto_20150606_2003'), ('filer', '0003_thumbnailoption'), ('filer', '0004_auto_20160328_1434'), ('filer', '0005_auto_20160623_1425'), ('filer', '0006_auto_20160623_1627'), ('filer', '0007_auto_20161016_1055'), ('filer', '0008_auto_20171117_1313'), ('filer', '0009_auto_20171220_1635'), ('filer', '0010_auto_20180414_2058'), ('filer', '0011_auto_20190418_0137'), ('filer', '0012_file_mime_type'), ('filer', '0013_image_width_height_to_float'), ('filer', '0014_folder_permission_choices'), ('filer', '0015_alter_file_owner_alter_file_polymorphic_ctype_and_more'), ('filer', '0016_alter_folder_index_together_remove_folder_level_and_more')]

initial = True

dependencies = [
('contenttypes', '0001_initial'),
('contenttypes', '0002_remove_content_type_name'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
('auth', '0001_initial'),
]

operations = [
migrations.CreateModel(
name='Clipboard',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='filer_clipboards', to=settings.AUTH_USER_MODEL, verbose_name='user')),
],
options={
'verbose_name': 'clipboard',
'verbose_name_plural': 'clipboards',
},
),
migrations.CreateModel(
name='Folder',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=255, verbose_name='name')),
('uploaded_at', models.DateTimeField(auto_now_add=True, verbose_name='uploaded at')),
('created_at', models.DateTimeField(auto_now_add=True, verbose_name='created at')),
('modified_at', models.DateTimeField(auto_now=True, verbose_name='modified at')),
('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='filer_owned_folders', to=settings.AUTH_USER_MODEL, verbose_name='owner')),
('parent', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='filer.folder', verbose_name='parent')),
],
options={
'ordering': ('name',),
'verbose_name': 'Folder',
'verbose_name_plural': 'Folders',
'permissions': (('can_use_directory_listing', 'Can use directory listing'),),
'unique_together': {('parent', 'name')},
},
bases=(models.Model, filer.models.mixins.IconsMixin),
),
migrations.CreateModel(
name='File',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('file', filer.fields.multistorage_file.MultiStorageFileField(blank=True, max_length=255, null=True, upload_to=filer.fields.multistorage_file.generate_filename_multistorage, verbose_name='file')),
('_file_size', models.BigIntegerField(blank=True, null=True, verbose_name='file size')),
('sha1', models.CharField(blank=True, default='', max_length=40, verbose_name='sha1')),
('has_all_mandatory_data', models.BooleanField(default=False, editable=False, verbose_name='has all mandatory data')),
('original_filename', models.CharField(blank=True, max_length=255, null=True, verbose_name='original filename')),
('name', models.CharField(blank=True, default='', max_length=255, verbose_name='name')),
('description', models.TextField(blank=True, null=True, verbose_name='description')),
('uploaded_at', models.DateTimeField(auto_now_add=True, verbose_name='uploaded at')),
('modified_at', models.DateTimeField(auto_now=True, verbose_name='modified at')),
('is_public', models.BooleanField(default=filer.models.filemodels.is_public_default, help_text='Disable any permission checking for this file. File will be publicly accessible to anyone.', verbose_name='Permissions disabled')),
('folder', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='all_files', to='filer.folder', verbose_name='folder')),
('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='owned_%(class)ss', to=settings.AUTH_USER_MODEL, verbose_name='owner')),
('polymorphic_ctype', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='polymorphic_%(app_label)s.%(class)s_set+', to='contenttypes.contenttype')),
('mime_type', models.CharField(default='application/octet-stream', help_text='MIME type of uploaded content', max_length=255, validators=[filer.models.filemodels.mimetype_validator])),
],
options={
'verbose_name': 'file',
'verbose_name_plural': 'files',
},
bases=(models.Model, filer.models.mixins.IconsMixin),
),
migrations.CreateModel(
name='ClipboardItem',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('clipboard', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='filer.clipboard', verbose_name='clipboard')),
('file', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='filer.file', verbose_name='file')),
],
options={
'verbose_name': 'clipboard item',
'verbose_name_plural': 'clipboard items',
},
),
migrations.CreateModel(
name='ThumbnailOption',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=100, verbose_name='name')),
('width', models.IntegerField(help_text='width in pixel.', verbose_name='width')),
('height', models.IntegerField(help_text='height in pixel.', verbose_name='height')),
('crop', models.BooleanField(default=True, verbose_name='crop')),
('upscale', models.BooleanField(default=True, verbose_name='upscale')),
],
options={
'ordering': ('width', 'height'),
'verbose_name': 'thumbnail option',
'verbose_name_plural': 'thumbnail options',
},
),
migrations.AddField(
model_name='clipboard',
name='files',
field=models.ManyToManyField(related_name='in_clipboards', through='filer.ClipboardItem', to='filer.File', verbose_name='files'),
),
migrations.CreateModel(
name='FolderPermission',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('type', models.SmallIntegerField(choices=[(0, 'all items'), (1, 'this item only'), (2, 'this item and all children')], default=0, verbose_name='type')),
('everybody', models.BooleanField(default=False, verbose_name='everybody')),
('can_edit', models.SmallIntegerField(blank=True, choices=[(None, 'inherit'), (1, 'allow'), (0, 'deny')], default=None, null=True, verbose_name='can edit')),
('can_read', models.SmallIntegerField(blank=True, choices=[(None, 'inherit'), (1, 'allow'), (0, 'deny')], default=None, null=True, verbose_name='can read')),
('can_add_children', models.SmallIntegerField(blank=True, choices=[(None, 'inherit'), (1, 'allow'), (0, 'deny')], default=None, null=True, verbose_name='can add children')),
('folder', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='filer.folder', verbose_name='folder')),
('group', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='filer_folder_permissions', to='auth.group', verbose_name='group')),
('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='filer_folder_permissions', to=settings.AUTH_USER_MODEL, verbose_name='user')),
],
options={
'verbose_name': 'folder permission',
'verbose_name_plural': 'folder permissions',
},
),
migrations.CreateModel(
name='Image',
fields=[
('file_ptr', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, related_name='%(app_label)s_%(class)s_file', serialize=False, to='filer.file')),
('_height', models.FloatField(blank=True, null=True)),
('_width', models.FloatField(blank=True, null=True)),
('date_taken', models.DateTimeField(blank=True, editable=False, null=True, verbose_name='date taken')),
('default_alt_text', models.CharField(blank=True, max_length=255, null=True, verbose_name='default alt text')),
('default_caption', models.CharField(blank=True, max_length=255, null=True, verbose_name='default caption')),
('author', models.CharField(blank=True, max_length=255, null=True, verbose_name='author')),
('must_always_publish_author_credit', models.BooleanField(default=False, verbose_name='must always publish author credit')),
('must_always_publish_copyright', models.BooleanField(default=False, verbose_name='must always publish copyright')),
('subject_location', models.CharField(blank=True, default='', max_length=64, verbose_name='subject location')),
],
options={
'swappable': 'FILER_IMAGE_MODEL',
'verbose_name': 'image',
'verbose_name_plural': 'images',
'default_manager_name': 'objects',
},
bases=('filer.file',),
),
]
2 changes: 1 addition & 1 deletion filer/migrations/0012_file_mime_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ class Migration(migrations.Migration):
name='mime_type',
field=models.CharField(default='application/octet-stream', help_text='MIME type of uploaded content', max_length=255, validators=[filer.models.filemodels.mimetype_validator]),
),
migrations.RunPython(guess_mimetypes, reverse_code=migrations.RunPython.noop),
migrations.RunPython(guess_mimetypes, reverse_code=migrations.RunPython.noop, elidable=True),
]
9 changes: 8 additions & 1 deletion filer/models/foldermodels.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from .. import settings as filer_settings
from . import mixins
from ..cache import get_folder_permission_cache, update_folder_permission_cache


class FolderPermissionManager(models.Manager):
Expand All @@ -34,6 +35,10 @@ def get_add_children_id_list(self, user):
def __get_id_list(self, user, attr):
if user.is_superuser or not filer_settings.FILER_ENABLE_PERMISSIONS:
return 'All'
cached_id_list = get_folder_permission_cache(user, attr)
if cached_id_list:
return cached_id_list

allow_list = set()
deny_list = set()
group_ids = user.groups.all().values_list('id', flat=True)
Expand Down Expand Up @@ -71,7 +76,9 @@ def __get_id_list(self, user, attr):
deny_list.update(perm.folder.get_descendants_ids())

# Deny has precedence over allow
return allow_list - deny_list
id_list = allow_list - deny_list
update_folder_permission_cache(user, attr, id_list)
return id_list


class Folder(models.Model, mixins.IconsMixin):
Expand Down
1 change: 1 addition & 0 deletions tests/requirements/django-main.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
-r base.txt

git+https://github.com/django/django@main#egg=Django
git+https://github.com/SmileyChris/easy-thumbnails@master#egg=easy-thumbnails
django_polymorphic>=3.1
1 change: 1 addition & 0 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
}
}
},
'THUMBNAIL_DEFAULT_STORAGE_ALIAS': 'default', # for the lack of any other storage defined
'SECRET_KEY': '__secret__',
'DEFAULT_AUTO_FIELD': 'django.db.models.AutoField',
}
Expand Down
2 changes: 2 additions & 0 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,8 @@ def test_with_permission_given_to_parent_folder(self):
can_edit=FolderPermission.ALLOW,
can_read=FolderPermission.ALLOW,
can_add_children=FolderPermission.ALLOW)
from filer.cache import clear_folder_permission_cache
clear_folder_permission_cache(self.staff_user)
response = self.client.get(
reverse('admin:filer-directory_listing',
kwargs={'folder_id': self.parent.id}))
Expand Down
Loading

0 comments on commit 01702d8

Please sign in to comment.