Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #7454 from edx/andya/user-api-transaction-handling

Browse Source 
Clean up transactional behavior in User API
This commit is contained in:
Christina Roberts
2015-03-25 15:08:21 -04:00
parent 8107f803f5 a9dce7c331
commit 3f07660d27
7 changed files with 105 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
openedx/core/djangoapps/course_groups/cohorts.py
View File

@@ -175,9 +175,7 @@ def get_cohorted_commentables(course_key):
@transaction.commit_on_success
def get_cohort(user, course_key, assign=True, use_cached=False):
"""
Given a Django user and a CourseKey, return the user's cohort in that
cohort.
"""Returns the user's cohort for the specified course.
The cohort for the user is cached for the duration of a request. Pass
use_cached=True to use the cached value instead of fetching from the

9
openedx/core/djangoapps/user_api/accounts/api.py
View File

@@ -7,7 +7,7 @@ from django.conf import settings
from django.core.validators import validate_email, validate_slug, ValidationError
from student.models import User, UserProfile, Registration
from student.views import validate_new_email, do_email_change_request
from student import views as student_views
from ..errors import (
AccountUpdateError, AccountValidationError, AccountUsernameInvalid, AccountPasswordInvalid,
@@ -92,7 +92,6 @@ def get_account_settings(requesting_user, username=None, configuration=None, vie
return visible_settings
@transaction.commit_on_success
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
def update_account_settings(requesting_user, update, username=None):
"""Update user account information.
@@ -154,7 +153,7 @@ def update_account_settings(requesting_user, update, username=None):
if read_only_fields:
for read_only_field in read_only_fields:
field_errors[read_only_field] = {
"developer_message": "This field is not editable via this API",
"developer_message": u"This field is not editable via this API",
"user_message": _(u"Field '{field_name}' cannot be edited.").format(field_name=read_only_field)
}
del update[read_only_field]
@@ -168,7 +167,7 @@ def update_account_settings(requesting_user, update, username=None):
# If the user asked to change email, validate it.
if changing_email:
try:
validate_new_email(existing_user, new_email)
student_views.validate_new_email(existing_user, new_email)
except ValueError as err:
field_errors["email"] = {
"developer_message": u"Error thrown from validate_new_email: '{}'".format(err.message),
@@ -206,7 +205,7 @@ def update_account_settings(requesting_user, update, username=None):
# And try to send the email change request if necessary.
if changing_email:
try:
do_email_change_request(existing_user, new_email)
student_views.do_email_change_request(existing_user, new_email)
except ValueError as err:
raise AccountUpdateError(
u"Error thrown from do_email_change_request: '{}'".format(err.message),

37
openedx/core/djangoapps/user_api/accounts/tests/test_views.py
View File

@@ -6,6 +6,7 @@ from mock import patch
from django.conf import settings
from django.core.urlresolvers import reverse
from django.test.testcases import TransactionTestCase
from rest_framework.test import APITestCase, APIClient
from student.tests.factories import UserFactory
@@ -509,3 +510,39 @@ class TestAccountAPI(UserAPITestCase):
error_response.data["developer_message"]
)
self.assertIsNone(error_response.data["user_message"])
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class TestAccountAPITransactions(TransactionTestCase):
"""
Tests the transactional behavior of the account API
"""
test_password = "test"
def setUp(self):
super(TestAccountAPITransactions, self).setUp()
self.client = APIClient()
self.user = UserFactory.create(password=self.test_password)
self.url = reverse("accounts_api", kwargs={'username': self.user.username})
@patch('student.views.do_email_change_request')
def test_update_account_settings_rollback(self, mock_email_change):
"""
Verify that updating account settings is transactional when a failure happens.
"""
# Send a PATCH request with updates to both profile information and email.
# Throw an error from the method that is used to process the email change request
# (this is the last thing done in the api method). Verify that the profile did not change.
mock_email_change.side_effect = [ValueError, "mock value error thrown"]
self.client.login(username=self.user.username, password=self.test_password)
old_email = self.user.email
json_data = {"email": "foo@bar.com", "gender": "o"}
response = self.client.patch(self.url, data=json.dumps(json_data), content_type="application/merge-patch+json")
self.assertEqual(400, response.status_code)
# Verify that GET returns the original preferences
response = self.client.get(self.url)
data = response.data
self.assertEqual(old_email, data["email"])
self.assertEqual(u"m", data["gender"])

4
openedx/core/djangoapps/user_api/accounts/views.py
View File

@@ -4,6 +4,7 @@ NOTE: this API is WIP and has not yet been approved. Do not use this API without
For more information, see:
https://openedx.atlassian.net/wiki/display/TNL/User+API
"""
from django.db import transaction
from rest_framework.views import APIView
from rest_framework.response import Response
from rest_framework import status
@@ -117,7 +118,8 @@ class AccountView(APIView):
else an error response with status code 415 will be returned.
"""
try:
update_account_settings(request.user, request.DATA, username=username)
with transaction.commit_on_success():
update_account_settings(request.user, request.DATA, username=username)
except (UserNotFound, UserNotAuthorized):
return Response(status=status.HTTP_404_NOT_FOUND)
except AccountValidationError as err:

15
openedx/core/djangoapps/user_api/preferences/api.py
View File

@@ -11,8 +11,9 @@ from pytz import UTC
from django.conf import settings
from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.db import transaction, IntegrityError
from django.db import IntegrityError
from django.utils.translation import ugettext as _
from django.utils.translation import ugettext_noop
from student.models import UserProfile
@@ -77,7 +78,6 @@ def get_user_preferences(requesting_user, username=None):
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
@transaction.commit_on_success
def update_user_preferences(requesting_user, update, username=None):
"""Update the user preferences for the given username.
@@ -138,7 +138,6 @@ def update_user_preferences(requesting_user, update, username=None):
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
@transaction.commit_on_success
def set_user_preference(requesting_user, preference_key, preference_value, username=None):
"""Update a user preference for the given username.
@@ -173,7 +172,6 @@ def set_user_preference(requesting_user, preference_key, preference_value, usern
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
@transaction.commit_on_success
def delete_user_preference(requesting_user, preference_key, username=None):
"""Deletes a user preference on behalf of a requesting user.
@@ -353,11 +351,12 @@ def validate_user_preference_serializer(serializer, preference_key, preference_v
PreferenceValidationError: the supplied key and/or value for a user preference are invalid.
"""
if preference_value is None or unicode(preference_value).strip() == '':
message = _(u"Preference '{preference_key}' cannot be set to an empty value.").format(
preference_key=preference_key
)
format_string = ugettext_noop(u"Preference '{preference_key}' cannot be set to an empty value.")
raise PreferenceValidationError({
preference_key: {"developer_message": message, "user_message": message}
preference_key: {
"developer_message": format_string.format(preference_key=preference_key),
"user_message": _(format_string).format(preference_key=preference_key)
}
})
if not serializer.is_valid():
developer_message = u"Value '{preference_value}' not valid for preference '{preference_key}': {error}".format(

50
openedx/core/djangoapps/user_api/preferences/tests/test_views.py
View File

@@ -6,9 +6,13 @@ Unit tests for preference APIs.
import unittest
import ddt
import json
from mock import patch
from django.core.urlresolvers import reverse
from django.conf import settings
from django.test.testcases import TransactionTestCase
from rest_framework.test import APIClient
from student.tests.factories import UserFactory
from ...accounts.tests.test_views import UserAPITestCase
from ..api import set_user_preference
@@ -288,6 +292,52 @@ class TestPreferencesAPI(UserAPITestCase):
)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class TestPreferencesAPITransactions(TransactionTestCase):
"""
Tests the transactional behavior of the preferences API
"""
test_password = "test"
def setUp(self):
super(TestPreferencesAPITransactions, self).setUp()
self.client = APIClient()
self.user = UserFactory.create(password=self.test_password)
self.url = reverse("preferences_api", kwargs={'username': self.user.username})
@patch('openedx.core.djangoapps.user_api.models.UserPreference.delete')
def test_update_preferences_rollback(self, delete_user_preference):
"""
Verify that updating preferences is transactional when a failure happens.
"""
# Create some test preferences values.
set_user_preference(self.user, "a", "1")
set_user_preference(self.user, "b", "2")
set_user_preference(self.user, "c", "3")
# Send a PATCH request with two updates and a delete. The delete should fail
# after one of the updates has happened, in which case the whole operation
# should be rolled back.
delete_user_preference.side_effect = [Exception, None]
self.client.login(username=self.user.username, password=self.test_password)
json_data = {
"a": "2",
"b": None,
"c": "1",
}
response = self.client.patch(self.url, data=json.dumps(json_data), content_type="application/merge-patch+json")
self.assertEqual(400, response.status_code)
# Verify that GET returns the original preferences
response = self.client.get(self.url)
expected_preferences = {
"a": "1",
"b": "2",
"c": "3",
}
self.assertEqual(expected_preferences, response.data)
@ddt.ddt
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class TestPreferencesDetailAPI(UserAPITestCase):

4
openedx/core/djangoapps/user_api/preferences/views.py
View File

@@ -10,6 +10,7 @@ from rest_framework import status
from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
from rest_framework import permissions
from django.db import transaction
from django.utils.translation import ugettext as _
from openedx.core.lib.api.parsers import MergePatchParser
@@ -88,7 +89,8 @@ class PreferencesView(APIView):
status=status.HTTP_400_BAD_REQUEST
)
try:
update_user_preferences(request.user, request.DATA, username=username)
with transaction.commit_on_success():
update_user_preferences(request.user, request.DATA, username=username)
except (UserNotFound, UserNotAuthorized):
return Response(status=status.HTTP_404_NOT_FOUND)
except PreferenceValidationError as error: