From 8d9297ea04f32c6fb27a658cf43527285d07a819 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 02:02:11 -0400 Subject: [PATCH] Some refactoring of how user info is copied over when enrollments are created. --- common/djangoapps/student/models.py | 156 +++++++++++++++++++++------- 1 file changed, 121 insertions(+), 35 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 5e10e39b51..1d5926f2be 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -17,6 +17,13 @@ We do a partial replication of: * User -- Askbot extends this and uses the extra fields, so we replicate only the stuff that comes with basic django_auth and ignore the rest.) +There are a couple different scenarios: + +1. There's an update of User or UserProfile -- replicate it to all Course DBs + that the user is enrolled in (found via CourseEnrollment). +2. There's a change in CourseEnrollment. We need to push copies of UserProfile, + CourseEnrollment, and the base fields in User + Migration Notes If you make changes to this model, be sure to create an appropriate migration @@ -247,53 +254,132 @@ def add_user_to_default_group(user, group): utg.users.add(User.objects.get(username=user)) utg.save() -################################# SIGNALS ###################################### +########################## REPLICATION SIGNALS ################################# +@receiver(post_save, sender=CourseEnrollment) +def replicate_enrollment_save(sender, **kwargs): + """This is called when a Student enrolls in a course. It has to do the + following: + + 1. Make sure the User is copied into the Course DB. It may already exist + (someone deleting and re-adding a course). This has to happen first or + the foreign key constraint breaks. + 2. Replicate the CourseEnrollment. + 3. Replicate the UserProfile. + """ + enrollment_obj = kwargs['instance'] + replicate_user(enrollment_obj.user, enrollment_obj.course_id) + replicate_model(CourseEnrollment.save, enrollment_obj.user_id, **kwargs) + replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + +@receiver(post_delete, sender=CourseEnrollment) +def replicate_enrollment_delete(sender, **kwargs): + enrollment_obj = kwargs['instance'] + return replicate_model(CourseEnrollment.delete, enrollment_obj.user_id, **kwargs) + +@receiver(post_save, sender=UserProfile) +def replicate_userprofile_save(sender, **kwargs): + """We just updated the UserProfile (say an update to the name), so push that + change to all Course DBs that we're enrolled in.""" + user_profile_obj = kwargs['instance'] + return replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + +######### Replication functions ######### +def replicate_user(portal_user, course_db_name): + """Replicate a User to the correct Course DB. This is more complicated than + it should be because Askbot extends the auth_user table and adds its own + fields. So we need to only push changes to the standard fields and leave + the rest alone so that Askbot can + """ + try: + # If the user exists in the Course DB, update the appropriate fields and + # save it back out to the Course DB. + course_user = User.objects.using(course_db_name).get(portal_user.id) + fields_to_copy = ["username", "first_name", "last_name", "email", + "password", "is_staff", "is_active", "is_superuser", + "last_login", "date_joined"] + for field in fields_to_copy: + setattr(course_user, field, getattr(portal_user, field)) + + mark_handled(course_user) + course_user.save(using=course_db_name) # Just being explicit. + + except User.DoesNotExist: + # Otherwise, just make a straight copy to the Course DB. + mark_handled(portal_user) + portal_user.save(using=course_db_name) + +def replicate_model(model_method, user_id, **kwargs): + """ + model_method is the model action that we want replicated. For instance, + UserProfile.save + """ + instance = kwargs['instance'] + if not should_replicate(instance): + return + + mark_handled(instance) + course_db_names = db_names_to_replicate_to(user_id) + log.debug("Replicating {0} for user {1} to DBs: {2}" + .format(model_method, user_id, course_db_names)) + + for db_name in course_db_names: + model_method(instance, using=db_name) + +######### Replication Helpers ######### def is_valid_course_id(course_id): """We check to both make sure that it's a valid course_id (and not 'default', or some other non-course DB name) and that we have a mapping for what database it belongs to.""" course_ids = set(course.id for course in modulestore().get_courses()) - return (course_id in course_ids) and (course_id in settings.DATABASES) + is_valid = (course_id in course_ids) and (course_id in settings.DATABASES) + if not is_valid: + log.error("{0} is not a valid DB to replicate to.".format(course_id)) + return is_valid def is_portal(): - """Are we in the portal pool? (in which case we'll have to replicate user - updates). Right now, that means we have more than one database defined.""" + """Are we in the portal pool? Only Portal servers are allowed to replicate + their changes. For now, only Portal servers see multiple DBs, so we use + that to decide.""" return len(settings.DATABASES) > 1 -def replicate_enrollment(instance_method, **kwargs): - log.debug("########## Enrollment replication called ############") - instance = kwargs['instance'] +def db_names_to_replicate_to(user_id): + """Return a list of DB names that this user_id is enrolled in.""" + return [c.course_id + for c in CourseEnrollment.objects.filter(user_id=user_id) + if is_valid_course_id(c.course_id)] +def marked_handled(instance): + """Have we marked this instance as being handled to avoid infinite loops + caused by saving models in post_save hooks for the same models?""" + return hasattr(instance, '_do_not_copy_to_course_db') + +def mark_handled(instance): + """You have to mark your instance with this function or else we'll go into + an infinite loop since we're putting listeners on Model saves/deletes and + the act of replication requires us to call the same model method. + + We create a _replicated attribute to differentiate the first save of this + model vs. the duplicate save we force on to the course database. Kind of + a hack -- suggestions welcome. + """ + instance._do_not_copy_to_course_db = True + +def should_replicate(instance): + """Should this instance be replicated? We need to be a Portal server and + the instance has to not have been marked_handled.""" + if marked_handled(instance): + # Basically, avoid an infinite loop. You should + log.debug("{0} should not be replicated because it's been marked") + return False if not is_portal(): - log.debug("replicate_enrollment triggered, but we're not a portal so " + - "we're not propogating") - return - - if not is_valid_course_id(instance.course_id): - log.error("Don't know where to replicate to for course_id: {0}" - .format(instance.course_id)) - return - - # We create a _replicated attribute to differentiate the first save of this - # model vs. the duplicate save we force on to the course database. - if hasattr(instance, '_replicated'): - log.debug("We've already replicated this -- stopping so we don't go " + - "into an infinite loop.") - return - instance._replicated = True - - # instance_method is either CourseEnrollment.save or CourseEnrollment.delete - # using is the entry in DATABASES we push to (we use course_ids for names) - instance_method(instance, using=instance.course_id) - -@receiver(post_save, sender=CourseEnrollment) -def replicate_enrollment_save(sender, **kwargs): - return replicate_enrollment(CourseEnrollment.save, **kwargs) - -@receiver(post_delete, sender=CourseEnrollment) -def replicate_enrollment_delete(sender, **kwargs): - return replicate_enrollment(CourseEnrollment.delete, **kwargs) + log.debug("{0} should not be replicated because we're not a portal." + .format(instance)) + return False + return True + + +