Commit 2c6437a2 by Don Mitchell Committed by Sarina Canelake

Fix performance regression

Remove course_from_id
don't fetch whole course when the root will do
don't fetch even the root if the id will do
check for definition.data == null
parent b073cfae
......@@ -4,9 +4,7 @@ Views for the course_mode module
import decimal
from django.core.urlresolvers import reverse
from django.http import (
HttpResponseBadRequest, Http404
)
from django.http import HttpResponseBadRequest
from django.shortcuts import redirect
from django.views.generic.base import View
from django.utils.translation import ugettext as _
......@@ -18,9 +16,9 @@ from edxmako.shortcuts import render_to_response
from course_modes.models import CourseMode
from courseware.access import has_access
from student.models import CourseEnrollment
from student.views import course_from_id
from verify_student.models import SoftwareSecurePhotoVerification
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore.django import modulestore
class ChooseModeView(View):
......@@ -54,7 +52,7 @@ class ChooseModeView(View):
donation_for_course = request.session.get("donation_for_course", {})
chosen_price = donation_for_course.get(course_key, None)
course = course_from_id(course_key)
course = modulestore().get_course(course_key)
context = {
"course_modes_choose_url": reverse("course_modes_choose", kwargs={'course_id': course_key.to_deprecated_string()}),
"modes": modes,
......@@ -80,7 +78,7 @@ class ChooseModeView(View):
# This is a bit redundant with logic in student.views.change_enrollement,
# but I don't really have the time to refactor it more nicely and test.
course = course_from_id(course_key)
course = modulestore().get_course(course_key)
if not has_access(user, 'enroll', course):
error_msg = _("Enrollment is closed")
return self.get(request, course_id, error=error_msg)
......
......@@ -589,7 +589,7 @@ def course_specific_login(request, course_id):
Dispatcher function for selecting the specific login method
required by the course
"""
course = student.views.course_from_id(course_id)
course = modulestore().get_course(course_id)
if not course:
# couldn't find the course, will just return vanilla signin page
return redirect_with_get('signin_user', request.GET)
......@@ -607,7 +607,7 @@ def course_specific_register(request, course_id):
Dispatcher function for selecting the specific registration method
required by the course
"""
course = student.views.course_from_id(course_id)
course = modulestore().get_course(course_id)
if not course:
# couldn't find the course, will just return vanilla registration page
......
......@@ -49,11 +49,10 @@ from verify_student.models import SoftwareSecurePhotoVerification, MidcourseReve
from certificates.models import CertificateStatuses, certificate_status_for_student
from dark_lang.models import DarkLangConfig
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from xmodule.modulestore import XML_MODULESTORE_TYPE
from collections import namedtuple
......@@ -128,11 +127,6 @@ def index(request, extra_context={}, user=AnonymousUser()):
return render_to_response('index.html', context)
def course_from_id(course_id):
"""Return the CourseDescriptor corresponding to this course_id"""
return modulestore().get_course(course_id)
def embargo(_request):
"""
Render the embargo page.
......@@ -242,7 +236,7 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set):
a student's dashboard.
"""
for enrollment in CourseEnrollment.enrollments_for_user(user):
course = course_from_id(enrollment.course_id)
course = modulestore().get_course(enrollment.course_id)
if course and not isinstance(course, ErrorDescriptor):
# if we are in a Microsite, then filter out anything that is not
......@@ -603,7 +597,7 @@ def change_enrollment(request):
# Make sure the course exists
# We don't do this check on unenroll, or a bad course id can't be unenrolled from
try:
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
except ItemNotFoundError:
log.warning("User {0} tried to enroll in non-existent course {1}"
.format(user.username, course_id))
......@@ -671,7 +665,7 @@ def _get_course_enrollment_domain(course_id):
@param course_id:
@return:
"""
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
if course is None:
return None
......
......@@ -165,7 +165,7 @@ class ModuleStoreRead(object):
pass
@abstractmethod
def get_course(self, course_id, depth=None):
def get_course(self, course_id, depth=0):
'''
Look for a specific course by its id (:class:`CourseKey`).
Returns the course descriptor, or None if not found.
......@@ -332,7 +332,7 @@ class ModuleStoreReadBase(ModuleStoreRead):
"""
return {}
def get_course(self, course_id, depth=None):
def get_course(self, course_id, depth=0):
"""
See ModuleStoreRead.get_course
......
......@@ -157,7 +157,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
return courses.values()
def get_course(self, course_key, depth=None):
def get_course(self, course_key, depth=0):
"""
returns the course module associated with the course_id. If no such course exists,
it returns None
......
......@@ -184,7 +184,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
if isinstance(data, basestring):
data = {'data': data}
mixed_class = self.mixologist.mix(class_)
data = self._convert_reference_fields_to_keys(mixed_class, location.course_key, data)
if data is not None:
data = self._convert_reference_fields_to_keys(mixed_class, location.course_key, data)
metadata = self._convert_reference_fields_to_keys(mixed_class, location.course_key, metadata)
kvs = MongoKeyValueStore(
data,
......@@ -350,7 +351,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
# call out to the DB
resultset = self.collection.find(query, record_filter)
# it's ok to keep these as urls b/c the overall cache is indexed by course_key and this
# it's ok to keep these as deprecated strings b/c the overall cache is indexed by course_key and this
# is a dictionary relative to that course
results_by_url = {}
root = None
......@@ -411,7 +412,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
# then look in any caching subsystem (e.g. memcached)
if self.metadata_inheritance_cache_subsystem is not None:
tree = self.metadata_inheritance_cache_subsystem.get(course_id, {})
tree = self.metadata_inheritance_cache_subsystem.get(unicode(course_id), {})
else:
logging.warning('Running MongoModuleStore without a metadata_inheritance_cache_subsystem. This is OK in localdev and testing environment. Not OK in production.')
......@@ -421,7 +422,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
# now write out computed tree to caching subsystem (e.g. memcached), if available
if self.metadata_inheritance_cache_subsystem is not None:
self.metadata_inheritance_cache_subsystem.set(course_id, tree)
self.metadata_inheritance_cache_subsystem.set(unicode(course_id), tree)
# now populate a request_cache, if available. NOTE, we are outside of the
# scope of the above if: statement so that after a memcache hit, it'll get
......@@ -590,7 +591,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
raise ItemNotFoundError(location)
return item
def get_course(self, course_key, depth=None):
def get_course(self, course_key, depth=0):
"""
Get the course with the given courseid (org/course/run)
"""
......
......@@ -319,7 +319,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
result.append(course_list[0])
return result
def get_course(self, course_id, depth=None):
def get_course(self, course_id, depth=0):
'''
Gets the course descriptor for the course identified by the locator
'''
......
......@@ -35,7 +35,7 @@ from course_modes.models import CourseMode
from open_ended_grading import open_ended_notifications
from student.models import UserTestGroup, CourseEnrollment
from student.views import course_from_id, single_course_reverification_info
from student.views import single_course_reverification_info
from util.cache import cache, cache_if_anonymous
from xblock.fragment import Fragment
from xmodule.modulestore.django import modulestore
......@@ -719,7 +719,7 @@ def fetch_reverify_banner_info(request, course_key):
if not user.id:
return reverifications
enrollment = CourseEnrollment.get_or_create_enrollment(request.user, course_key)
course = course_from_id(course_key)
course = modulestore().get_course(course_key)
info = single_course_reverification_info(user, course, enrollment)
if info:
reverifications[info.status].append(info)
......
......@@ -72,7 +72,6 @@ from student.models import (
unique_id_for_user,
anonymous_id_for_user
)
from student.views import course_from_id
import track.views
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
......@@ -1620,7 +1619,7 @@ def _do_unenroll_students(course_key, students, email_students=False):
settings.SITE_NAME
)
if email_students:
course = course_from_id(course_key)
course = modulestore().get_course(course_key)
#Composition of email
d = {'site_name': stripped_site_name,
'course': course}
......
......@@ -21,12 +21,9 @@ from django.core.urlresolvers import reverse
from model_utils.managers import InheritanceManager
from xmodule.modulestore.django import modulestore
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.exceptions import ItemNotFoundError
from course_modes.models import CourseMode
from edxmako.shortcuts import render_to_string
from student.views import course_from_id
from student.models import CourseEnrollment, unenroll_done
from util.query import use_read_replica_if_available
from xmodule_django.models import CourseKeyField
......@@ -332,7 +329,7 @@ class PaidCourseRegistration(OrderItem):
Returns the order item
"""
# First a bunch of sanity checks
course = course_from_id(course_id) # actually fetch the course to make sure it exists, use this to
course = modulestore().get_course(course_id) # actually fetch the course to make sure it exists, use this to
# throw errors if it doesn't
if not course:
log.error("User {} tried to add non-existent course {} to cart id {}"
......@@ -528,7 +525,7 @@ class CertificateItem(OrderItem):
item.status = order.status
item.qty = 1
item.unit_cost = cost
course_name = course_from_id(course_id).display_name
course_name = modulestore().get_course(course_id).display_name
item.line_desc = _("Certificate of Achievement, {mode_name} for course {course}").format(mode_name=mode_info.name,
course=course_name)
item.currency = currency
......@@ -544,7 +541,7 @@ class CertificateItem(OrderItem):
try:
verification_attempt = SoftwareSecurePhotoVerification.active_for_user(self.course_enrollment.user)
verification_attempt.submit()
except Exception as e:
except Exception:
log.exception(
"Could not submit verification attempt for enrollment {}".format(self.course_enrollment)
)
......@@ -560,7 +557,7 @@ class CertificateItem(OrderItem):
@property
def single_item_receipt_context(self):
course = course_from_id(self.course_id)
course = modulestore().get_course(self.course_id)
return {
"course_id": self.course_id,
"course_name": course.display_name_with_default,
......
......@@ -23,7 +23,7 @@ from django.contrib.auth.decorators import login_required
from course_modes.models import CourseMode
from student.models import CourseEnrollment
from student.views import course_from_id, reverification_info
from student.views import reverification_info
from shoppingcart.models import Order, CertificateItem
from shoppingcart.processors.CyberSource import (
get_signed_purchase_params, get_purchase_endpoint
......@@ -36,6 +36,7 @@ import ssencrypt
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.locations import SlashSeparatedCourseKey
from .exceptions import WindowExpiredException
from xmodule.modulestore.django import modulestore
log = logging.getLogger(__name__)
......@@ -83,7 +84,7 @@ class VerifyView(View):
else:
chosen_price = verify_mode.min_price
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
context = {
"progress_state": progress_state,
"user_full_name": request.user.profile.name,
......@@ -133,7 +134,7 @@ class VerifiedView(View):
verify_mode.min_price
)
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
context = {
"course_id": course_id.to_deprecated_string(),
"course_modes_choose_url": reverse('course_modes_choose', kwargs={'course_id': course_id.to_deprecated_string()}),
......@@ -271,7 +272,6 @@ def results_callback(request):
# If this is a reverification, log an event
if attempt.window:
course_id = attempt.window.course_id
course = course_from_id(course_id)
course_enrollment = CourseEnrollment.get_or_create_enrollment(attempt.user, course_id)
course_enrollment.emit_event(EVENT_NAME_USER_REVERIFICATION_REVIEWED_BY_SOFTWARESECURE)
......@@ -288,7 +288,7 @@ def show_requirements(request, course_id):
return redirect(reverse('dashboard'))
upgrade = request.GET.get('upgrade', False)
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
context = {
"course_id": course_id.to_deprecated_string(),
"course_modes_choose_url": reverse("course_modes_choose", kwargs={'course_id': course_id.to_deprecated_string()}),
......@@ -372,7 +372,7 @@ class MidCourseReverifyView(View):
display this view
"""
course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
course = course_from_id(course_id)
course = modulestore().get_course(course_id)
course_enrollment = CourseEnrollment.get_or_create_enrollment(request.user, course_id)
course_enrollment.update_enrollment(mode="verified")
course_enrollment.emit_event(EVENT_NAME_USER_ENTERED_MIDCOURSE_REVERIFY_VIEW)
......@@ -440,7 +440,7 @@ def midcourse_reverify_dash(request):
course_enrollment_pairs = []
for enrollment in CourseEnrollment.enrollments_for_user(user):
try:
course_enrollment_pairs.append((course_from_id(enrollment.course_id), enrollment))
course_enrollment_pairs.append((modulestore().get_course(enrollment.course_id), enrollment))
except ItemNotFoundError:
log.error("User {0} enrolled in non-existent course {1}"
.format(user.username, enrollment.course_id))
......
<%! from django.utils.translation import ugettext as _ %>
<%! from student.views import course_from_id %>
<%inherit file="../main.html" />
<%block name="bodyclass">register verification-process step-confirmation</%block>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment