Updated docs, tests, tox config

django-guardian · Feb 11, 2013 · 24ad366 · 24ad366
1 parent ffcf671
commit 24ad366
Show file tree

Hide file tree

Showing 15 changed files with 186 additions and 51 deletions.
diff --git a/.gitignore b/.gitignore
@@ -13,6 +13,7 @@ dist
 .coverage
 .hgignore
 .tox
+.ropeproject
 
 example_project/media
 example_project/conf/*.py

diff --git a/benchmarks/models.py b/benchmarks/models.py
@@ -1,6 +1,21 @@
 from django.db import models
+from guardian.models import UserObjectPermissionBase
+from guardian.models import GroupObjectPermissionBase
 
 
 class TestModel(models.Model):
     name = models.CharField(max_length=128)
 
+
+
+class DirectUser(UserObjectPermissionBase):
+    content_object = models.ForeignKey('TestDirectModel')
+
+
+class DirectGroup(GroupObjectPermissionBase):
+    content_object = models.ForeignKey('TestDirectModel')
+
+
+class TestDirectModel(models.Model):
+    name = models.CharField(max_length=128)
+
diff --git a/benchmarks/run_benchmarks.py b/benchmarks/run_benchmarks.py
@@ -1,4 +1,8 @@
 #!/usr/bin/env python
+"""
+This benchmark package should be treated as work-in-progress, not a production
+ready benchmarking solution for django-guardian.
+"""
 import datetime
 import os
 import random
@@ -26,15 +30,18 @@
     'django.contrib.admin',
     'django.contrib.sites',
     'guardian',
+    'benchmarks',
 )
 
 from utils import show_settings
-from django.contrib.auth.models import User
+from django.contrib.auth.models import User, Group
+from django.utils.termcolors import colorize
 from benchmarks.models import TestModel
+from benchmarks.models import TestDirectModel
 
-USERS_COUNT = 20
-OBJECTS_COUNT = 100
-OBJECTS_WIHT_PERMS_COUNT = 100
+USERS_COUNT = 50
+OBJECTS_COUNT = 1000
+OBJECTS_WIHT_PERMS_COUNT = 1000
 
 def random_string(length=25, chars=string.ascii_letters+string.digits):
     return ''.join(random.choice(chars) for i in range(length))
@@ -72,34 +79,42 @@ def wrapper(*args, **kwargs):
                 call.finish = datetime.datetime.now()
                 func.calls.append(call)
                 if self.action:
-                    print(" -> [%s] Done (Total time: %s)" % (self.action, 
+                    print(" -> [%s] Done (Total time: %s)" % (self.action,
                         call.delta()))
         return wrapper
 
 
 class Benchmark(object):
 
-    def __init__(self, users_count, objects_count, objects_with_perms_count):
+    def __init__(self, name, users_count, objects_count,
+            objects_with_perms_count, model):
+        self.name = name
         self.users_count = users_count
         self.objects_count = objects_count
         self.objects_with_perms_count = objects_with_perms_count
-
-        self.Model = TestModel
-        self.perm = 'auth.change_testmodel'
+
+        self.Model = model
+        self.perm = 'auth.change_%s' % model._meta.module_name
+
+    def info(self, msg):
+        print colorize(msg + '\n', fg='green')
 
     def prepare_db(self):
         from django.core.management import call_command
         call_command('syncdb', interactive=False)
 
+        for model in [User, Group, self.Model]:
+            model.objects.all().delete()
+
     @Timed("Creating users")
     def create_users(self):
-        User.objects.bulk_create(User(username=random_string().capitalize())
+        User.objects.bulk_create(User(id=x, username=random_string().capitalize())
             for x in range(self.users_count))
 
     @Timed("Creating objects")
     def create_objects(self):
         Model = self.Model
-        Model.objects.bulk_create(Model(name=random_string(20))
+        Model.objects.bulk_create(Model(id=x, name=random_string(20))
             for x in range(self.objects_count))
 
     @Timed("Grant permissions")
@@ -126,6 +141,9 @@ def check_perm(self, user, obj, perm):
 
     @Timed("Benchmark")
     def main(self):
+        self.info('=' * 80)
+        self.info(self.name.center(80))
+        self.info('=' * 80)
         self.prepare_db()
         self.create_users()
         self.create_objects()
@@ -135,9 +153,13 @@ def main(self):
 
 def main():
     show_settings(settings, 'benchmarks')
-    benchmark = Benchmark(USERS_COUNT, OBJECTS_COUNT, OBJECTS_WIHT_PERMS_COUNT)
+    benchmark = Benchmark('Direct relations benchmark',
+        USERS_COUNT, OBJECTS_COUNT, OBJECTS_WIHT_PERMS_COUNT, TestDirectModel)
     benchmark.main()
 
+    benchmark = Benchmark('Generic relations benchmark',
+        USERS_COUNT, OBJECTS_COUNT, OBJECTS_WIHT_PERMS_COUNT, TestModel)
+    benchmark.main()
 
 if __name__ == '__main__':
     main()

diff --git a/benchmarks/settings.py b/benchmarks/settings.py
@@ -26,9 +26,10 @@
 
 DATABASES = {
     'default': {
-        'ENGINE': 'django.db.backends.sqlite3',
-        'NAME': ':memory:',
-        'TEST_NAME': ':memory:',
+        'ENGINE': 'django.db.backends.postgresql_psycopg2',
+        'NAME': 'guardian_benchmarks',
+        'USER': 'guardian_bench',
+        'PASSWORD': 'guardian_bench',
     },
 }
 
diff --git a/docs/userguide/index.rst b/docs/userguide/index.rst
@@ -10,5 +10,6 @@ User Guide
     check
     remove
     admin-integration
+    performance
     caveats
 
diff --git a/docs/userguide/performance.rst b/docs/userguide/performance.rst
@@ -0,0 +1,111 @@
+.. _performance:
+
+Performance tunning
+===================
+
+It is important to remember that by default ``django-guardian`` uses generic
+foreign keys to retain relation with any Django model. For most cases it's
+probably good enough, however if we have a lot of queries being spanned and
+our database seems to be choking it might be a good choice to use *direct*
+foreign keys. Let's start with quick overview of how generic solution work and
+then we will move on to the tunning part.
+
+
+Default, generic solution
+-------------------------
+
+``django-guardian`` comes with two models: :model:`UserObjectPermission` and
+:model:`GroupObjectPermission`. They both have same, generic way of pointing to
+other models:
+
+- ``content_type`` field telling what table (model class) target permission
+  references to (``ContentType`` instance)
+- ``object_pk`` field storing value of target model instance primary key
+- ``content_object`` field being a ``GenericForeignKey``. Actually, it is not
+  a foreign key in standard, relational database meaning - it is simply a proxy
+  that can retrieve proper model instance being targeted by two previous fields
+
+.. seealso::
+
+    https://docs.djangoproject.com/en/1.4/ref/contrib/contenttypes/#generic-relations
+
+Let's consider following model:
+
+.. code-block:: python
+
+    class Project(models.Model):
+        name = models.CharField(max_length=128, unique=True)
+
+
+In order to add a *change_project* permission for *joe* user we would use
+:ref:`api-shortcuts-assign` shortcut:
+
+.. code-block:: python
+
+    >>> from guardian.shortcuts import assign
+    >>> project = Project.objects.get(name='Foobar')
+    >>> joe = User.objects.get(username='joe')
+    >>> assign('change_project', joe, project)
+
+What it really does is: create an instance of :model:`UserObjectPermission`.
+Something similar to:
+
+.. code-block:: python
+
+    >>> content_type = ContentType.objects.get_for_model(Project)
+    >>> perm = Permission.objects.get(content_type__app_label='app',
+    ...     codename='change_project')
+    >>> UserObjectPermission.objects.create(user=joe, content_type=content_type,
+    ...     permission=perm, object_pk=project.pk)
+
+
+
+As there are no real foreing keys pointing at the target model this solution
+might not be enough for all cases. In example if we try to build an issues
+tracking service and we'd like to be able to support thousends of users and
+their project/tickets, object level permission checks can be slow with this
+generic solution.
+
+
+.. _performance-direct-fk:
+
+Direct foreign keys
+-------------------
+
+.. versionadded:: 1.3
+
+In order to make our permission checks faster we can use direct foreign key
+solution. It actually is very simple to setup - we need to declare two new
+models next to our ``Project`` model, one for ``User`` and one for ``Group``
+models:
+
+.. code-block:: python
+
+    from guardian.models import UserObjectPermissionBase
+    from guardian.models import GroupObjectPermissionBase
+
+    class Project(models.Model):
+        name = models.CharField(max_length=128, unique=True)
+
+    class ProjectUserObjectPermission(UserObjectPermissionBase):
+        content_object = models.ForeignKey(Project)
+
+    class ProjectGroupObjectPermission(GroupObjectPermissionBase):
+        content_object = models.ForeignKey(Project)
+
+
+.. important::
+   Name of the ``ForeignKey`` field is important and it should be
+   ``content_object`` as underlying queries depends on it.
+
+
+from now on ``guardian`` will figure out that ``Project`` model has direct
+relation for user/group object permissions and will use those models. It is
+also possible to use only user or only group based direct relation, however it
+is discouraged (it's not consistent and might be a quick road to hell from the
+mainteinence point of view, especially).
+
+.. note::
+   By defining direct relation models we can also tweak that object permission
+   model, i.e. by adding some fields
+
diff --git a/example_project/settings.py b/example_project/settings.py
@@ -34,7 +34,6 @@
     'guardian',
     'guardian.tests.testapp',
     'posts',
-    'taggit',
 )
 if 'GRAPPELLI' in os.environ:
     try:

diff --git a/guardian/models.py b/guardian/models.py
@@ -8,7 +8,6 @@
 from guardian.managers import UserObjectPermissionManager
 from guardian.managers import GroupObjectPermissionManager
 from guardian.utils import get_anonymous_user
-from guardian.utils import ClassProperty
 
 
 class BaseObjectPermission(models.Model):
@@ -35,16 +34,6 @@ def save(self, *args, **kwargs):
                 % (self.permission.content_type, content_type))
         return super(BaseObjectPermission, self).save(*args, **kwargs)
 
-    #@ClassProperty
-    #@classmethod
-    #def _unique_together_attrs(cls):
-        #first = 'user' if hasattr(cls, 'user') else 'group'
-        #if isinstance(cls.content_object, GenericForeignKey):
-            #last = 'object_pk'
-        #else:
-            #last = 'content_object'
-        #return [first, 'permission', last]
-
 
 class BaseGenericObjectPermission(models.Model):
     content_type = models.ForeignKey(ContentType)
@@ -65,12 +54,12 @@ class UserObjectPermissionBase(BaseObjectPermission):
 
     class Meta:
         abstract = True
-        # TODO: set properly unique_together
-        #unique_together = ['user', 'permission', 'object_pk']
+        unique_together = ['user', 'permission', 'content_object']
 
 
 class UserObjectPermission(UserObjectPermissionBase, BaseGenericObjectPermission):
-    pass
+    class Meta:
+        unique_together = ['user', 'permission', 'object_pk']
 
 
 class GroupObjectPermissionBase(BaseObjectPermission):
@@ -83,12 +72,12 @@ class GroupObjectPermissionBase(BaseObjectPermission):
 
     class Meta:
         abstract = True
-        # TODO: set properly unique_together
-        #unique_together = ['group', 'permission', 'object_pk']
+        unique_together = ['group', 'permission', 'content_object']
 
 
 class GroupObjectPermission(GroupObjectPermissionBase, BaseGenericObjectPermission):
-    pass
+    class Meta:
+        unique_together = ['group', 'permission', 'object_pk']
 
 
 # Prototype User and Group methods

diff --git a/guardian/tests/__init__.py b/guardian/tests/__init__.py
@@ -5,8 +5,8 @@
 from core_test import *
 from custompkmodel_test import *
 from decorators_test import *
+from direct_rel_test import *
 from forms_test import *
-from managers_test import *
 from orphans_test import *
 from other_test import *
 from utils_test import *

diff --git a/guardian/tests/testapp/models.py b/guardian/tests/testapp/models.py
@@ -1,12 +1,6 @@
 from datetime import datetime
 from django.db import models
 from guardian.models import UserObjectPermissionBase, GroupObjectPermissionBase
-from taggit.managers import TaggableManager
-from taggit.models import TaggedItemBase
-
-
-class TaggedProject(TaggedItemBase):
-    content_object = models.ForeignKey('Project')
 
 
 class ProjectUserObjectPermission(UserObjectPermissionBase):
@@ -21,8 +15,6 @@ class Project(models.Model):
     name = models.CharField(max_length=128, unique=True)
     created_at = models.DateTimeField(default=datetime.now)
 
-    tags = TaggableManager(through=TaggedProject)
-
     class Meta:
         get_latest_by = 'created_at'
 

diff --git a/guardian/tests/utils_test.py b/guardian/tests/utils_test.py
@@ -1,3 +1,5 @@
+from mock import Mock
+from mock import patch
 from django.test import TestCase
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.auth.models import User, Group, AnonymousUser

diff --git a/guardian/testsettings.py b/guardian/testsettings.py
@@ -1,4 +1,6 @@
 import os
+import random
+import string
 
 DEBUG = False
 
@@ -12,6 +14,7 @@
     'django.contrib.admin',
     'django.contrib.messages',
     'guardian',
+    'guardian.tests.testapp',
 )
 
 AUTHENTICATION_BACKENDS = (
@@ -36,6 +39,8 @@
     os.path.join(os.path.dirname(__file__), 'tests', 'templates'),
 )
 
+SECRET_KEY = ''.join([random.choice(string.printable) for x in range(40)])
+
 # Database specific
 
 if os.environ.get('GUARDIAN_TEST_DB_BACKEND') == 'mysql':