make user and group management more robust

- remove TaskResultInline and subclasses
- add custom perform_delete_selected action to UserAdmin and GroupAdmin
- properly clean asynchronous tasks in rabbitmq
- wrap user operations in transactions

gnuviechadmin/osusers/admin.py

@@ -25,30 +25,6 @@ class ShadowInline(admin.TabularInline):
     can_delete = False
 
 
-class TaskResultInline(admin.TabularInline):
-    can_delete = False
-    extra = 0
-    readonly_fields = ['task_uuid', 'task_name', 'is_finished', 'is_success',
-                       'state', 'result_body']
-
-    def get_queryset(self, request):
-        qs = super(TaskResultInline, self).get_queryset(request)
-        for entry in qs:
-            entry.update_taskstatus()
-        return qs
-
-    def has_add_permission(self, request, obj=None):
-        return False
-
-
-class UserTaskResultInline(TaskResultInline):
-    model = UserTaskResult
-
-
-class GroupTaskResultInline(TaskResultInline):
-    model = GroupTaskResult
-
-
 class UserCreationForm(forms.ModelForm):
     """
     A form for creating system users.
@@ -91,9 +67,10 @@ class UserCreationForm(forms.ModelForm):
 
 
 class UserAdmin(admin.ModelAdmin):
-    inlines = [AdditionalGroupInline, ShadowInline, UserTaskResultInline]
+    actions = ['perform_delete_selected']
     readonly_fields = ['uid']
     add_form = UserCreationForm
+    inlines = [AdditionalGroupInline, ShadowInline]
 
     add_fieldsets = (
         (None, {
@@ -115,37 +92,82 @@ class UserAdmin(admin.ModelAdmin):
         defaults.update(kwargs)
         return super(UserAdmin, self).get_form(request, obj, **defaults)
 
-    def get_inline_instances(self, request, obj=None):
+    def get_readonly_fields(self, request, obj=None):
-        if obj is None:
+        if obj:
-            return []
+            return ['uid']
-        return super(UserAdmin, self).get_inline_instances(request, obj)
+        return []
+
+    def perform_delete_selected(self, request, queryset):
+        for user in queryset.all():
+            user.delete()
+    perform_delete_selected.short_description = _('Delete selected users')
+
+    def get_actions(self, request):
+        actions = super(UserAdmin, self).get_actions(request)
+        if 'delete_selected' in actions:
+            del actions['delete_selected']
+        return actions
 
 
 class GroupAdmin(admin.ModelAdmin):
-    inlines = [GroupTaskResultInline]
+    actions = ['perform_delete_selected']
 
     def get_inline_instances(self, request, obj=None):
         if obj is None:
             return []
         return super(GroupAdmin, self).get_inline_instances(request, obj)
 
+    def perform_delete_selected(self, request, queryset):
+        for group in queryset.all():
+            group.delete()
+    perform_delete_selected.short_description = _('Delete selected groups')
 
-class DeleteTaskResultAdmin(admin.ModelAdmin):
+    def get_actions(self, request):
-    readonly_fields = ['task_uuid', 'task_name', 'modeltype', 'modelname',
+        actions = super(GroupAdmin, self).get_actions(request)
-                       'is_finished', 'is_success', 'state', 'result_body']
+        if 'delete_selected' in actions:
-    list_display = ('task_uuid', 'task_name', 'modeltype', 'modelname',
+            del actions['delete_selected']
-                    'is_finished', 'state')
+        return actions
 
+
+class TaskResultAdmin(admin.ModelAdmin):
     def has_add_permission(self, request, obj=None):
         return False
 
+    def has_delete_permission(self, request, obj=None):
+        return obj is None or obj.is_finished
+
     def get_queryset(self, request):
-        qs = super(DeleteTaskResultAdmin, self).get_queryset(request)
+        qs = super(TaskResultAdmin, self).get_queryset(request)
         for entry in qs:
             entry.update_taskstatus()
         return qs
 
 
+class DeleteTaskResultAdmin(TaskResultAdmin):
+    readonly_fields = ['task_uuid', 'task_name', 'modeltype', 'modelname',
+                       'is_finished', 'is_success', 'state', 'result_body']
+    list_display = ('task_uuid', 'task_name', 'modeltype', 'modelname',
+                    'is_finished', 'state')
+
+
+class GroupTaskResultAdmin(TaskResultAdmin):
+    readonly_fields = [
+        'task_uuid', 'task_name', 'group', 'is_finished', 'is_success',
+        'state', 'result_body'
+    ]
+    list_display = ('task_uuid', 'task_name', 'group', 'is_finished', 'state')
+
+
+class UserTaskResultAdmin(TaskResultAdmin):
+    readonly_fields = [
+        'task_uuid', 'task_name', 'user', 'is_finished', 'is_success', 'state',
+        'result_body'
+    ]
+    list_display = ('task_uuid', 'task_name', 'user', 'is_finished', 'state')
+
+
 admin.site.register(Group, GroupAdmin)
 admin.site.register(User, UserAdmin)
 admin.site.register(DeleteTaskResult, DeleteTaskResultAdmin)
+admin.site.register(GroupTaskResult, GroupTaskResultAdmin)
+admin.site.register(UserTaskResult, UserTaskResultAdmin)

gnuviechadmin/osusers/models.py

@@ -1,3 +1,5 @@
+from __future__ import unicode_literals
+
 from datetime import date
 import os
 
@@ -42,18 +44,17 @@ class TaskResult(TimeStampedModel, models.Model):
         abstract = True
 
     def _set_result_fields(self, asyncresult):
-        if asyncresult.ready():
+        if not self.is_finished:
+            result = asyncresult.get(no_ack=False)
             self.is_finished = True
             self.is_success = asyncresult.state == 'SUCCESS'
-            self.result_body = str(asyncresult.result)
             self.state = asyncresult.state
-            asyncresult.get(no_ack=False)
+            self.result_body = str(result)
 
     def update_taskstatus(self):
-        if not self.is_finished:
+        asyncresult = AsyncResult(self.task_uuid)
-            asyncresult = AsyncResult(self.task_uuid)
+        self._set_result_fields(asyncresult)
-            self._set_result_fields(asyncresult)
+        self.save()
-            self.save()
 
 
 class GroupManager(models.Manager):
@@ -84,6 +85,7 @@ class Group(TimeStampedModel, models.Model):
     def __str__(self):
         return '{0} ({1})'.format(self.groupname, self.gid)
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         super(Group, self).save(*args, **kwargs)
         GroupTaskResult.objects.create_grouptaskresult(
@@ -93,6 +95,7 @@ class Group(TimeStampedModel, models.Model):
         )
         return self
 
+    @transaction.atomic
     def delete(self, *args, **kwargs):
         DeleteTaskResult.objects.create_deletetaskresult(
             'group', self.groupname,
@@ -125,6 +128,7 @@ class DeleteTaskResultManager(TaskResultManager):
         return taskresult
 
 
+@python_2_unicode_compatible
 class DeleteTaskResult(TaskResult):
 
     modeltype = models.CharField(max_length=20, db_index=True)
@@ -132,6 +136,11 @@ class DeleteTaskResult(TaskResult):
 
     objects = DeleteTaskResultManager()
 
+    def __str__(self):
+        return "{task_uuid} {task_name} {modeltype} {modelname}".format(
+            task_uuid=self.task_uuid, task_name=self.task_name,
+            modeltype=self.modeltype, modelname=self.modelname)
+
 
 class GroupTaskResultManager(TaskResultManager):
 
@@ -145,12 +154,18 @@ class GroupTaskResultManager(TaskResultManager):
         return taskresult
 
 
+@python_2_unicode_compatible
 class GroupTaskResult(TaskResult):
 
     group = models.ForeignKey(Group)
 
     objects = GroupTaskResultManager()
 
+    def __str__(self):
+        return "{task_uuid} {task_name} {group}".format(
+            task_uuid=self.task_uuid, task_name=self.task_name,
+            group=self.group)
+
 
 class UserManager(models.Manager):
 
@@ -215,6 +230,7 @@ class User(TimeStampedModel, models.Model):
     def __str__(self):
         return '{0} ({1})'.format(self.username, self.uid)
 
+    @transaction.atomic
     def set_password(self, password):
         if hasattr(self, 'shadow'):
             self.shadow.set_password(password)
@@ -232,6 +248,7 @@ class User(TimeStampedModel, models.Model):
             commit=True
         )
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         UserTaskResult.objects.create_usertaskresult(
             self,
@@ -243,6 +260,7 @@ class User(TimeStampedModel, models.Model):
         )
         return super(User, self).save(*args, **kwargs)
 
+    @transaction.atomic
     def delete(self, *args, **kwargs):
         for group in [
             ag.group for ag in AdditionalGroup.objects.filter(user=self)
@@ -274,15 +292,21 @@ class UserTaskResultManager(TaskResultManager):
         return taskresult
 
 
+@python_2_unicode_compatible
 class UserTaskResult(TaskResult):
 
     user = models.ForeignKey(User)
 
     objects = UserTaskResultManager()
 
+    def __str__(self):
+        return "{task_uuid} {task_name} {user}".format(
+            task_uuid=self.task_uuid, task_name=self.task_name, user=self.user)
+
 
 class ShadowManager(models.Manager):
 
+    @transaction.atomic
     def create_shadow(self, user, password):
         changedays = (timezone.now().date() - date(1970, 1, 1)).days
         shadow = self.create(
@@ -356,6 +380,7 @@ class AdditionalGroup(TimeStampedModel, models.Model):
         if self.user.group == self.group:
             raise ValidationError(CANNOT_USE_PRIMARY_GROUP_AS_ADDITIONAL)
 
+    @transaction.atomic
     def save(self, *args, **kwargs):
         GroupTaskResult.objects.create_grouptaskresult(
             self.group,
@@ -365,6 +390,7 @@ class AdditionalGroup(TimeStampedModel, models.Model):
         )
         super(AdditionalGroup, self).save(*args, **kwargs)
 
+    @transaction.atomic
     def delete(self, *args, **kwargs):
         DeleteTaskResult.objects.create_deletetaskresult(
             'usergroup',