Explorar el Código

HUE-906 [useradmin] Lazily create profiles

Profile creation should be lazy instead of using signals.
This improves migration and upgrade of Hue database.
Profiles should be created for users created through hue UI.
Abraham Elmahrek hace 13 años
padre
commit
e4480a2d82

+ 7 - 8
apps/useradmin/src/useradmin/models.py

@@ -131,7 +131,11 @@ def get_profile(user):
   if hasattr(user, "_cached_userman_profile"):
   if hasattr(user, "_cached_userman_profile"):
     return user._cached_userman_profile
     return user._cached_userman_profile
   else:
   else:
-    profile = UserProfile.objects.get(user=user)
+    # Lazily create profile.
+    try:
+      profile = UserProfile.objects.get(user=user)
+    except UserProfile.DoesNotExist, e:
+      profile = create_profile_for_user(user)
     user._cached_userman_profile = profile
     user._cached_userman_profile = profile
     return profile
     return profile
 
 
@@ -148,15 +152,10 @@ def create_profile_for_user(user):
   p.home_directory = "/user/%s" % p.user.username
   p.home_directory = "/user/%s" % p.user.username
   try:
   try:
     p.save()
     p.save()
+    return p
   except:
   except:
     LOG.debug("Failed to automatically create user profile.", exc_info=True)
     LOG.debug("Failed to automatically create user profile.", exc_info=True)
-
-def create_user_signal_handler(sender, **kwargs):
-  if kwargs['created']:
-    create_profile_for_user(kwargs['instance'])
-
-# Create a user profile every time a user gets created.
-models.signals.post_save.connect(create_user_signal_handler, sender=auth_models.User)
+    return None
 
 
 class LdapGroup(models.Model):
 class LdapGroup(models.Model):
   """
   """

+ 21 - 1
apps/useradmin/src/useradmin/tests.py

@@ -185,6 +185,15 @@ def test_default_group():
   assert_false(Group.objects.filter(name='test_default').exists())
   assert_false(Group.objects.filter(name='test_default').exists())
   assert_true(Group.objects.filter(name='new_default').exists())
   assert_true(Group.objects.filter(name='new_default').exists())
 
 
+def test_get_profile():
+  # Ensure profiles are created after get_profile is called.
+  reset_all_users()
+  reset_all_groups()
+  c = make_logged_in_client(username='test', password='test', is_superuser=True)
+  assert_equal(0, UserProfile.objects.count())
+  p = get_profile(User.objects.get(username='test'))
+  assert_equal(1, UserProfile.objects.count())
+
 def test_group_admin():
 def test_group_admin():
   reset_all_users()
   reset_all_users()
   reset_all_groups()
   reset_all_groups()
@@ -320,6 +329,8 @@ def test_user_admin():
   assert_true(FUNNY_NAME_QUOTED in response.content)
   assert_true(FUNNY_NAME_QUOTED in response.content)
   assert_true(len(response.context["users"]) > 1)
   assert_true(len(response.context["users"]) > 1)
   assert_true("Hue Users" in response.content)
   assert_true("Hue Users" in response.content)
+  # Validate profile is created.
+  assert_true(UserProfile.objects.filter(user__username=FUNNY_NAME).exists())
 
 
   # Need to give access to the user for the rest of the test
   # Need to give access to the user for the rest of the test
   group = Group.objects.create(name="test-group")
   group = Group.objects.create(name="test-group")
@@ -364,12 +375,21 @@ def test_user_admin():
               "Inactivated user gets redirected to login page")
               "Inactivated user gets redirected to login page")
 
 
   # Delete that regular user
   # Delete that regular user
-  funny_profile = UserProfile.objects.get(user=test_user)
+  funny_profile = get_profile(test_user)
   response = c_su.post('/useradmin/users/delete/%s' % (FUNNY_NAME_QUOTED,))
   response = c_su.post('/useradmin/users/delete/%s' % (FUNNY_NAME_QUOTED,))
   assert_equal(302, response.status_code)
   assert_equal(302, response.status_code)
   assert_false(User.objects.filter(username=FUNNY_NAME).exists())
   assert_false(User.objects.filter(username=FUNNY_NAME).exists())
   assert_false(UserProfile.objects.filter(id=funny_profile.id).exists())
   assert_false(UserProfile.objects.filter(id=funny_profile.id).exists())
 
 
+  # Make sure that user deletion works if the user has never performed a request.
+  User.objects.create(username=FUNNY_NAME, password='test')
+  assert_true(User.objects.filter(username=FUNNY_NAME).exists())
+  assert_false(UserProfile.objects.filter(user__username=FUNNY_NAME).exists())
+  response = c_su.post('/useradmin/users/delete/%s' % (FUNNY_NAME_QUOTED,))
+  assert_equal(302, response.status_code)
+  assert_false(User.objects.filter(username=FUNNY_NAME).exists())
+  assert_false(UserProfile.objects.filter(user__username=FUNNY_NAME).exists())
+
   # You shouldn't be able to create a user without a password
   # You shouldn't be able to create a user without a password
   response = c_su.post('/useradmin/users/new', dict(username="test"))
   response = c_su.post('/useradmin/users/new', dict(username="test"))
   assert_true("You must specify a password when creating a new user." in response.content)
   assert_true("You must specify a password when creating a new user." in response.content)

+ 8 - 2
apps/useradmin/src/useradmin/views.py

@@ -75,8 +75,12 @@ def delete_user(request, username):
         if username == request.user.username:
         if username == request.user.username:
           raise PopupException(_("You cannot remove yourself."), error_code=401)
           raise PopupException(_("You cannot remove yourself."), error_code=401)
         user = User.objects.get(username=username)
         user = User.objects.get(username=username)
-        user_profile = UserProfile.objects.get(user=user)
-        user_profile.delete()
+        # Since profiles are lazily created, they may not exist.
+        try:
+          user_profile = UserProfile.objects.get(user=user)
+          user_profile.delete()
+        except UserProfile.DoesNotExist, e:
+          pass
         user.delete()
         user.delete()
       finally:
       finally:
         __users_lock.release()
         __users_lock.release()
@@ -144,6 +148,8 @@ def edit_user(request, username=None):
     if form.is_valid(): # All validation rules pass
     if form.is_valid(): # All validation rules pass
       if instance is None:
       if instance is None:
         instance = form.save()
         instance = form.save()
+        # Create profile for new users.
+        get_profile(instance)
       else:
       else:
         #
         #
         # Check for 3 more conditions:
         # Check for 3 more conditions: