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

Merge pull request #461 from MITx/feature/victor/multi-run-policy

Browse Source 
Keep each course's descriptors separate
This commit is contained in:
Calen Pennington
2012-08-20 12:35:55 -07:00
parent 9bde5e7a4b 55b1a59c40
commit 259e21364f
23 changed files with 221 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/djangoapps/student/views.py
View File

@@ -102,7 +102,7 @@ def main_index(request, extra_context={}, user=None):
def course_from_id(course_id):
"""Return the CourseDescriptor corresponding to this course_id"""
course_loc = CourseDescriptor.id_to_location(course_id)
return modulestore().get_item(course_loc)
return modulestore().get_instance(course_id, course_loc)
def press(request):

4
common/lib/xmodule/xmodule/course_module.py
View File

@@ -134,6 +134,10 @@ class CourseDescriptor(SequenceDescriptor):
'all_descriptors' : all_descriptors,}
@staticmethod
def make_id(org, course, url_name):
return '/'.join([org, course, url_name])
@staticmethod
def id_to_location(course_id):
'''Convert the given course_id (org/course/name) to a location object.

10
common/lib/xmodule/xmodule/modulestore/__init__.py
View File

@@ -223,6 +223,13 @@ class ModuleStore(object):
"""
raise NotImplementedError
def get_instance(self, course_id, location):
"""
Get an instance of this location, with policy for course_id applied.
TODO (vshnayder): this may want to live outside the modulestore eventually
"""
raise NotImplementedError
def get_item_errors(self, location):
"""
Return a list of (msg, exception-or-None) errors that the modulestore
@@ -331,7 +338,8 @@ class ModuleStoreBase(ModuleStore):
and datastores.
"""
# check that item is present and raise the promised exceptions if needed
self.get_item(location)
# TODO (vshnayder): post-launch, make errors properties of items
#self.get_item(location)
errorlog = self._get_errorlog(location)
return errorlog.errors

7
common/lib/xmodule/xmodule/modulestore/mongo.py
View File

@@ -217,6 +217,13 @@ class MongoModuleStore(ModuleStoreBase):
item = self._find_one(location)
return self._load_items([item], depth)[0]
def get_instance(self, course_id, location):
"""
TODO (vshnayder): implement policy tracking in mongo.
For now, just delegate to get_item and ignore policy.
"""
return self.get_item(location)
def get_items(self, location, depth=0):
items = self.collection.find(
location_to_query(location),

59
common/lib/xmodule/xmodule/modulestore/xml.py
View File

@@ -3,6 +3,7 @@ import logging
import os
import re
from collections import defaultdict
from fs.osfs import OSFS
from importlib import import_module
from lxml import etree
@@ -33,7 +34,7 @@ def clean_out_mako_templating(xml_string):
return xml_string
class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
def __init__(self, xmlstore, org, course, course_dir,
def __init__(self, xmlstore, course_id, course_dir,
policy, error_tracker, **kwargs):
"""
A class that handles loading from xml. Does some munging to ensure that
@@ -43,6 +44,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
"""
self.unnamed_modules = 0
self.used_slugs = set()
self.org, self.course, self.url_name = course_id.split('/')
def process_xml(xml):
"""Takes an xml string, and returns a XModuleDescriptor created from
@@ -80,21 +82,24 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
xml_data.set('url_name', slug)
descriptor = XModuleDescriptor.load_from_xml(
etree.tostring(xml_data), self, org,
course, xmlstore.default_class)
etree.tostring(xml_data), self, self.org,
self.course, xmlstore.default_class)
#log.debug('==> importing descriptor location %s' %
# repr(descriptor.location))
descriptor.metadata['data_dir'] = course_dir
xmlstore.modules[descriptor.location] = descriptor
xmlstore.modules[course_id][descriptor.location] = descriptor
if xmlstore.eager:
descriptor.get_children()
return descriptor
render_template = lambda: ''
load_item = xmlstore.get_item
# TODO (vshnayder): we are somewhat architecturally confused in the loading code:
# load_item should actually be get_instance, because it expects the course-specific
# policy to be loaded. For now, just add the course_id here...
load_item = lambda location: xmlstore.get_instance(course_id, location)
resources_fs = OSFS(xmlstore.data_dir / course_dir)
MakoDescriptorSystem.__init__(self, load_item, resources_fs,
@@ -127,7 +132,7 @@ class XMLModuleStore(ModuleStoreBase):
self.eager = eager
self.data_dir = path(data_dir)
self.modules = {} # location -> XModuleDescriptor
self.modules = defaultdict(dict) # course_id -> dict(location -> XModuleDescriptor)
self.courses = {} # course_dir -> XModuleDescriptor for the course
if default_class is None:
@@ -236,14 +241,24 @@ class XMLModuleStore(ModuleStoreBase):
tracker(msg)
course = course_dir
url_name = course_data.get('url_name')
url_name = course_data.get('url_name', course_data.get('slug'))
if url_name:
policy_path = self.data_dir / course_dir / 'policies' / '{0}.json'.format(url_name)
policy = self.load_policy(policy_path, tracker)
else:
policy = {}
# VS[compat] : 'name' is deprecated, but support it for now...
if course_data.get('name'):
url_name = Location.clean(course_data.get('name'))
tracker("'name' is deprecated for module xml. Please use "
"display_name and url_name.")
else:
raise ValueError("Can't load a course without a 'url_name' "
"(or 'name') set. Set url_name.")
system = ImportSystem(self, org, course, course_dir, policy, tracker)
course_id = CourseDescriptor.make_id(org, course, url_name)
system = ImportSystem(self, course_id, course_dir, policy, tracker)
course_descriptor = system.process_xml(etree.tostring(course_data))
@@ -257,6 +272,27 @@ class XMLModuleStore(ModuleStoreBase):
return course_descriptor
def get_instance(self, course_id, location, depth=0):
"""
Returns an XModuleDescriptor instance for the item at
location, with the policy for course_id. (In case two xml
dirs have different content at the same location, return the
one for this course_id.)
If any segment of the location is None except revision, raises
xmodule.modulestore.exceptions.InsufficientSpecificationError
If no object is found at that location, raises
xmodule.modulestore.exceptions.ItemNotFoundError
location: Something that can be passed to Location
"""
location = Location(location)
try:
return self.modules[course_id][location]
except KeyError:
raise ItemNotFoundError(location)
def get_item(self, location, depth=0):
"""
Returns an XModuleDescriptor instance for the item at location.
@@ -271,11 +307,8 @@ class XMLModuleStore(ModuleStoreBase):
location: Something that can be passed to Location
"""
location = Location(location)
try:
return self.modules[location]
except KeyError:
raise ItemNotFoundError(location)
raise NotImplementedError("XMLModuleStores can't guarantee that definitions"
" are unique. Use get_instance.")
def get_courses(self, depth=0):

31
common/lib/xmodule/xmodule/modulestore/xml_importer.py
View File

@@ -22,21 +22,22 @@ def import_from_xml(store, data_dir, course_dirs=None, eager=True,
eager=eager,
course_dirs=course_dirs
)
for module in module_store.modules.itervalues():
for course_id in module_store.modules.keys():
for module in module_store.modules[course_id].itervalues():
# TODO (cpennington): This forces import to overrite the same items.
# This should in the future create new revisions of the items on import
try:
store.create_item(module.location)
except DuplicateItemError:
log.exception('Item already exists at %s' % module.location.url())
pass
if 'data' in module.definition:
store.update_item(module.location, module.definition['data'])
if 'children' in module.definition:
store.update_children(module.location, module.definition['children'])
# NOTE: It's important to use own_metadata here to avoid writing
# inherited metadata everywhere.
store.update_metadata(module.location, dict(module.own_metadata))
# TODO (cpennington): This forces import to overrite the same items.
# This should in the future create new revisions of the items on import
try:
store.create_item(module.location)
except DuplicateItemError:
log.exception('Item already exists at %s' % module.location.url())
pass
if 'data' in module.definition:
store.update_item(module.location, module.definition['data'])
if 'children' in module.definition:
store.update_children(module.location, module.definition['children'])
# NOTE: It's important to use own_metadata here to avoid writing
# inherited metadata everywhere.
store.update_metadata(module.location, dict(module.own_metadata))
return module_store

1
common/lib/xmodule/xmodule/tests/__init__.py
View File

@@ -8,6 +8,7 @@
import unittest
import os
import fs
import fs.osfs
import json
import json

12
common/lib/xmodule/xmodule/tests/test_export.py
View File

@@ -84,20 +84,22 @@ class RoundTripTestCase(unittest.TestCase):
strip_filenames(exported_course)
self.assertEquals(initial_course, exported_course)
self.assertEquals(initial_course.id, exported_course.id)
course_id = initial_course.id
print "Checking key equality"
self.assertEquals(sorted(initial_import.modules.keys()),
sorted(second_import.modules.keys()))
self.assertEquals(sorted(initial_import.modules[course_id].keys()),
sorted(second_import.modules[course_id].keys()))
print "Checking module equality"
for location in initial_import.modules.keys():
for location in initial_import.modules[course_id].keys():
print "Checking", location
if location.category == 'html':
print ("Skipping html modules--they can't import in"
" final form without writing files...")
continue
self.assertEquals(initial_import.modules[location],
second_import.modules[location])
self.assertEquals(initial_import.modules[course_id][location],
second_import.modules[course_id][location])
def setUp(self):

45
common/lib/xmodule/xmodule/tests/test_import.py
View File

@@ -207,3 +207,48 @@ class ImportTestCase(unittest.TestCase):
check_for_key(key, c)
check_for_key('graceperiod', course)
def test_policy_loading(self):
"""Make sure that when two courses share content with the same
org and course names, policy applies to the right one."""
def get_course(name):
print "Importing {0}".format(name)
modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=[name])
courses = modulestore.get_courses()
self.assertEquals(len(courses), 1)
return courses[0]
toy = get_course('toy')
two_toys = get_course('two_toys')
self.assertEqual(toy.url_name, "2012_Fall")
self.assertEqual(two_toys.url_name, "TT_2012_Fall")
toy_ch = toy.get_children()[0]
two_toys_ch = two_toys.get_children()[0]
self.assertEqual(toy_ch.display_name, "Overview")
self.assertEqual(two_toys_ch.display_name, "Two Toy Overview")
def test_definition_loading(self):
"""When two courses share the same org and course name and
both have a module with the same url_name, the definitions shouldn't clash.
TODO (vshnayder): once we have a CMS, this shouldn't
happen--locations should uniquely name definitions. But in
our imperfect XML world, it can (and likely will) happen."""
modulestore = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy', 'two_toys'])
toy_id = "edX/toy/2012_Fall"
two_toy_id = "edX/toy/TT_2012_Fall"
location = Location(["i4x", "edX", "toy", "video", "Welcome"])
toy_video = modulestore.get_instance(toy_id, location)
two_toy_video = modulestore.get_instance(two_toy_id, location)
self.assertEqual(toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh8")
self.assertEqual(two_toy_video.metadata['youtube'], "1.0:p2Q6BrNhdh9")

1
common/test/data/two_toys/README Normal file
View File

@@ -0,0 +1 @@
A copy of the toy course, with different metadata. Used to test policy loading for course with identical org course fields and shared content.

4
common/test/data/two_toys/chapter/Overview.xml Normal file
View File

@@ -0,0 +1,4 @@
<chapter>
<videosequence url_name="Toy_Videos"/>
<video url_name="Welcome"/>
</chapter>

1
common/test/data/two_toys/course.xml Normal file
View File

@@ -0,0 +1 @@
<course url_name="TT_2012_Fall" org="edX" course="toy"/>

3
common/test/data/two_toys/course/TT_2012_Fall.xml Normal file
View File

@@ -0,0 +1,3 @@
<course display_name="Toy Course" graceperiod="2 days 5 hours 59 minutes 59 seconds" start="2015-07-17T12:00">
<chapter url_name="Overview"/>
</course>

23
common/test/data/two_toys/policies/TT_2012_Fall.json Normal file
View File

@@ -0,0 +1,23 @@
{
"course/TT_2012_Fall": {
"graceperiod": "2 days 5 hours 59 minutes 59 seconds",
"start": "2015-07-17T12:00",
"display_name": "Two Toys Course"
},
"chapter/Overview": {
"display_name": "Two Toy Overview"
},
"videosequence/Toy_Videos": {
"display_name": "Toy Videos",
"format": "Lecture Sequence"
},
"html/toylab": {
"display_name": "Toy lab"
},
"video/Video_Resources": {
"display_name": "Video Resources"
},
"video/Welcome": {
"display_name": "Welcome"
}
}

1
common/test/data/two_toys/video/Video_Resources.xml Normal file
View File

@@ -0,0 +1 @@
<video youtube="1.0:1bK-WdDi6Qw" display_name="Video Resources"/>

1
common/test/data/two_toys/video/Welcome.xml Normal file
View File

@@ -0,0 +1 @@
<video youtube="1.0:p2Q6BrNhdh9" display_name="Welcome"/>

3
common/test/data/two_toys/videosequence/Toy_Videos.xml Normal file
View File

@@ -0,0 +1,3 @@
<videosequence display_name="Toy Videos" format="Lecture Sequence">
<video url_name="Video_Resources"/>
</videosequence>

2
lms/djangoapps/courseware/courses.py
View File

@@ -25,7 +25,7 @@ def get_course_by_id(course_id):
"""
try:
course_loc = CourseDescriptor.id_to_location(course_id)
return modulestore().get_item(course_loc)
return modulestore().get_instance(course_id, course_loc)
except (KeyError, ItemNotFoundError):
raise Http404("Course not found.")

5
lms/djangoapps/courseware/grades.py
View File

@@ -9,6 +9,7 @@ from django.conf import settings
from models import StudentModuleCache
from module_render import get_module, get_instance_module
from xmodule import graders
from xmodule.course_module import CourseDescriptor
from xmodule.graders import Score
from models import StudentModule
@@ -63,8 +64,10 @@ def grade(student, request, course, student_module_cache=None):
scores = []
# TODO: We need the request to pass into here. If we could forgo that, our arguments
# would be simpler
course_id = CourseDescriptor.location_to_id(course.location)
section_module = get_module(student, request,
section_descriptor.location, student_module_cache)
section_descriptor.location, student_module_cache,
course_id)
if section_module is None:
# student doesn't have access to this module, or something else
# went wrong.

9
lms/djangoapps/courseware/management/commands/check_course.py
View File

@@ -1,5 +1,7 @@
import os.path
# THIS COMMAND IS OUT OF DATE
from lxml import etree
from django.core.management.base import BaseCommand
@@ -72,13 +74,17 @@ class Command(BaseCommand):
# TODO: use args as list of files to check. Fix loading to work for other files.
print "This command needs updating before use"
return
"""
sample_user = User.objects.all()[0]
print "Attempting to load courseware"
# TODO (cpennington): Get coursename in a legitimate way
course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012'
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(sample_user, modulestore().get_item(course_location))
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
sample_user, modulestore().get_item(course_location))
course = get_module(sample_user, None, course_location, student_module_cache)
to_run = [
@@ -96,3 +102,4 @@ class Command(BaseCommand):
print 'Courseware passes all checks!'
else:
print "Courseware fails some checks"
"""

19
lms/djangoapps/courseware/management/commands/clean_xml.py
View File

@@ -124,9 +124,11 @@ def check_roundtrip(course_dir):
print "======== ideally there is no diff above this ======="
def clean_xml(course_dir, export_dir):
def clean_xml(course_dir, export_dir, force):
(ok, course) = import_with_checks(course_dir)
if ok:
if ok or force:
if not ok:
print "WARNING: Exporting despite errors"
export(course, export_dir)
check_roundtrip(export_dir)
else:
@@ -138,11 +140,18 @@ class Command(BaseCommand):
help = """Imports specified course.xml, validate it, then exports in
a canonical format.
Usage: clean_xml PATH-TO-COURSE-DIR PATH-TO-OUTPUT-DIR
Usage: clean_xml PATH-TO-COURSE-DIR PATH-TO-OUTPUT-DIR [force]
If 'force' is specified as the last argument, exports even if there
were import errors.
"""
def handle(self, *args, **options):
if len(args) != 2:
n = len(args)
if n < 2 or n > 3:
print Command.help
return
clean_xml(args[0], args[1])
force = False
if n == 3 and args[2] == 'force':
force = True
clean_xml(args[0], args[1], force)

34
lms/djangoapps/courseware/module_render.py
View File

@@ -71,13 +71,13 @@ def toc_for_course(user, request, course, active_chapter, active_section, course
'''
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2)
course = get_module(user, request, course.location, student_module_cache, course_id=course_id)
course = get_module(user, request, course.location, student_module_cache, course_id)
chapters = list()
for chapter in course.get_display_items():
hide_from_toc = chapter.metadata.get('hide_from_toc','false').lower() == 'true'
if hide_from_toc:
continue
continue
sections = list()
for section in chapter.get_display_items():
@@ -131,7 +131,7 @@ def get_section(course_module, chapter, section):
return section_module
def get_module(user, request, location, student_module_cache, position=None, course_id=None):
def get_module(user, request, location, student_module_cache, course_id, position=None):
''' Get an instance of the xmodule class identified by location,
setting the state based on an existing StudentModule, or creating one if none
exists.
@@ -141,21 +141,14 @@ def get_module(user, request, location, student_module_cache, position=None, cou
- request : current django HTTPrequest
- location : A Location-like object identifying the module to load
- student_module_cache : a StudentModuleCache
- course_id : the course_id in the context of which to load module
- position : extra information from URL for user-specified
position within module
Returns: xmodule instance
'''
descriptor = modulestore().get_item(location)
# NOTE:
# A 'course_id' is understood to be the triplet (org, course, run), for example
# (MITx, 6.002x, 2012_Spring).
# At the moment generic XModule does not contain enough information to replicate
# the triplet (it is missing 'run'), so we must pass down course_id
if course_id is None:
course_id = descriptor.location.course_id # Will NOT produce (org, course, run) for non-CourseModule's
descriptor = modulestore().get_instance(course_id, location)
# Short circuit--if the user shouldn't have access, bail without doing any work
if not has_access(user, descriptor, 'load'):
@@ -207,7 +200,7 @@ def get_module(user, request, location, student_module_cache, position=None, cou
Delegate to get_module. It does an access check, so may return None
"""
return get_module(user, request, location,
student_module_cache, position, course_id=course_id)
student_module_cache, course_id, position)
# TODO (cpennington): When modules are shared between courses, the static
# prefix is going to have to be specific to the module, not the directory
@@ -325,7 +318,7 @@ def xqueue_callback(request, course_id, userid, id, dispatch):
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
user, modulestore().get_item(id), depth=0, select_for_update=True)
instance = get_module(user, request, id, student_module_cache)
instance = get_module(user, request, id, student_module_cache, course_id)
if instance is None:
log.debug("No module {0} for user {1}--access denied?".format(id, user))
raise Http404
@@ -361,7 +354,7 @@ def xqueue_callback(request, course_id, userid, id, dispatch):
return HttpResponse("")
def modx_dispatch(request, dispatch=None, id=None, course_id=None):
def modx_dispatch(request, dispatch, location, course_id):
''' Generic view for extensions. This is where AJAX calls go.
Arguments:
@@ -370,7 +363,8 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None):
- dispatch -- the command string to pass through to the module's handle_ajax call
(e.g. 'problem_reset'). If this string contains '?', only pass
through the part before the first '?'.
- id -- the module id. Used to look up the XModule instance
- location -- the module location. Used to look up the XModule instance
- course_id -- defines the course context for this request.
'''
# ''' (fix emacs broken parsing)
@@ -392,12 +386,14 @@ def modx_dispatch(request, dispatch=None, id=None, course_id=None):
return HttpResponse(json.dumps({'success': file_too_big_msg}))
p[fileinput_id] = inputfiles
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id))
instance = get_module(request.user, request, id, student_module_cache, course_id=course_id)
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
request.user, modulestore().get_item(course_id, location))
instance = get_module(request.user, request, location, student_module_cache, course_id)
if instance is None:
# Either permissions just changed, or someone is trying to be clever
# and load something they shouldn't have access to.
log.debug("No module {0} for user {1}--access denied?".format(id, user))
log.debug("No module {0} for user {1}--access denied?".format(location, user))
raise Http404
instance_module = get_instance_module(request.user, instance, student_module_cache)

10
lms/djangoapps/courseware/views.py
View File

@@ -64,7 +64,7 @@ def courses(request):
'''
Render "find courses" page. The course selection work is done in courseware.courses.
'''
universities = get_courses_by_university(request.user,
universities = get_courses_by_university(request.user,
domain=request.META.get('HTTP_HOST'))
return render_to_response("courses.html", {'universities': universities})
@@ -139,7 +139,7 @@ def index(request, course_id, chapter=None, section=None,
section_descriptor)
module = get_module(request.user, request,
section_descriptor.location,
student_module_cache, course_id=course_id)
student_module_cache, course_id)
if module is None:
# User is probably being clever and trying to access something
# they don't have access to.
@@ -277,9 +277,11 @@ def progress(request, course_id, student_id=None):
student = User.objects.get(id=int(student_id))
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course)
course_module = get_module(request.user, request, course.location, student_module_cache, course_id=course_id)
course_module = get_module(request.user, request, course.location,
student_module_cache, course_id)
courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache)
courseware_summary = grades.progress_summary(student, course_module,
course.grader, student_module_cache)
grade_summary = grades.grade(request.user, request, course, student_module_cache)
context = {'course': course,