Merge pull request #8902 from edx/things-to-tack-on-rc
Things to tack on rc
This commit is contained in:
@@ -7,6 +7,7 @@ Sample invocation: ./manage.py export_convert_format mycourse.tar.gz ~/newformat
|
||||
import os
|
||||
from path import path
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from django.conf import settings
|
||||
|
||||
from tempfile import mkdtemp
|
||||
import tarfile
|
||||
@@ -32,8 +33,8 @@ class Command(BaseCommand):
|
||||
output_path = args[1]
|
||||
|
||||
# Create temp directories to extract the source and create the target archive.
|
||||
temp_source_dir = mkdtemp()
|
||||
temp_target_dir = mkdtemp()
|
||||
temp_source_dir = mkdtemp(dir=settings.DATA_DIR)
|
||||
temp_target_dir = mkdtemp(dir=settings.DATA_DIR)
|
||||
try:
|
||||
extract_source(source_archive, temp_source_dir)
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ Test for export_convert_format.
|
||||
"""
|
||||
from unittest import TestCase
|
||||
from django.core.management import call_command, CommandError
|
||||
from django.conf import settings
|
||||
from tempfile import mkdtemp
|
||||
import shutil
|
||||
from path import path
|
||||
@@ -18,7 +19,7 @@ class ConvertExportFormat(TestCase):
|
||||
""" Common setup. """
|
||||
super(ConvertExportFormat, self).setUp()
|
||||
|
||||
self.temp_dir = mkdtemp()
|
||||
self.temp_dir = mkdtemp(dir=settings.DATA_DIR)
|
||||
self.addCleanup(shutil.rmtree, self.temp_dir)
|
||||
self.data_dir = path(__file__).realpath().parent / 'data'
|
||||
self.version0 = self.data_dir / "Version0_drafts.tar.gz"
|
||||
@@ -52,8 +53,8 @@ class ConvertExportFormat(TestCase):
|
||||
"""
|
||||
Helper function for determining if 2 archives are equal.
|
||||
"""
|
||||
temp_dir_1 = mkdtemp()
|
||||
temp_dir_2 = mkdtemp()
|
||||
temp_dir_1 = mkdtemp(dir=settings.DATA_DIR)
|
||||
temp_dir_2 = mkdtemp(dir=settings.DATA_DIR)
|
||||
try:
|
||||
extract_source(file1, temp_dir_1)
|
||||
extract_source(file2, temp_dir_2)
|
||||
|
||||
@@ -209,6 +209,19 @@ class ImportTestCase(CourseTestCase):
|
||||
|
||||
return outside_tar
|
||||
|
||||
def _edx_platform_tar(self):
|
||||
"""
|
||||
Tarfile with file that extracts to edx-platform directory.
|
||||
|
||||
Extracting this tarfile in directory <dir> will also put its contents
|
||||
directly in <dir> (rather than <dir/tarname>).
|
||||
"""
|
||||
outside_tar = self.unsafe_common_dir / "unsafe_file.tar.gz"
|
||||
with tarfile.open(outside_tar, "w:gz") as tar:
|
||||
tar.addfile(tarfile.TarInfo(os.path.join(os.path.abspath("."), "a_file")))
|
||||
|
||||
return outside_tar
|
||||
|
||||
def test_unsafe_tar(self):
|
||||
"""
|
||||
Check that safety measure work.
|
||||
@@ -233,6 +246,12 @@ class ImportTestCase(CourseTestCase):
|
||||
try_tar(self._symlink_tar())
|
||||
try_tar(self._outside_tar())
|
||||
try_tar(self._outside_tar2())
|
||||
try_tar(self._edx_platform_tar())
|
||||
|
||||
# test trying to open a tar outside of the normal data directory
|
||||
with self.settings(DATA_DIR='/not/the/data/dir'):
|
||||
try_tar(self._edx_platform_tar())
|
||||
|
||||
# Check that `import_status` returns the appropriate stage (i.e.,
|
||||
# either 3, indicating all previous steps are completed, or 0,
|
||||
# indicating no upload in progress)
|
||||
@@ -294,13 +313,19 @@ class ImportTestCase(CourseTestCase):
|
||||
self.assertIn(test_block3.url_name, children)
|
||||
self.assertIn(test_block4.url_name, children)
|
||||
|
||||
extract_dir = path(tempfile.mkdtemp())
|
||||
extract_dir = path(tempfile.mkdtemp(dir=settings.DATA_DIR))
|
||||
# the extract_dir needs to be passed as a relative dir to
|
||||
# import_library_from_xml
|
||||
extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR)
|
||||
|
||||
try:
|
||||
tar = tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz')
|
||||
safetar_extractall(tar, extract_dir)
|
||||
with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar:
|
||||
safetar_extractall(tar, extract_dir)
|
||||
library_items = import_library_from_xml(
|
||||
self.store, self.user.id,
|
||||
settings.GITHUB_REPO_ROOT, [extract_dir / 'library'],
|
||||
self.store,
|
||||
self.user.id,
|
||||
settings.GITHUB_REPO_ROOT,
|
||||
[extract_dir_relative / 'library'],
|
||||
load_error_modules=False,
|
||||
static_content_store=contentstore(),
|
||||
target_id=lib_key
|
||||
|
||||
@@ -39,6 +39,7 @@ INSTALLED_APPS += ('django_extensions',)
|
||||
TEST_ROOT = REPO_ROOT / "test_root" # pylint: disable=no-value-for-parameter
|
||||
GITHUB_REPO_ROOT = (TEST_ROOT / "data").abspath()
|
||||
LOG_DIR = (TEST_ROOT / "log").abspath()
|
||||
DATA_DIR = TEST_ROOT / "data"
|
||||
|
||||
# Configure modulestore to use the test folder within the repo
|
||||
update_module_store_settings(
|
||||
|
||||
@@ -65,6 +65,7 @@ TEST_ROOT = path('test_root')
|
||||
STATIC_ROOT = TEST_ROOT / "staticfiles"
|
||||
|
||||
GITHUB_REPO_ROOT = TEST_ROOT / "data"
|
||||
DATA_DIR = TEST_ROOT / "data"
|
||||
COMMON_TEST_DATA_ROOT = COMMON_ROOT / "test" / "data"
|
||||
|
||||
# For testing "push to lms"
|
||||
|
||||
@@ -587,6 +587,12 @@ class XModuleMixin(XModuleFields, XBlock):
|
||||
|
||||
if field.scope.user == UserScope.ONE:
|
||||
field._del_cached_value(self) # pylint: disable=protected-access
|
||||
# not the most elegant way of doing this, but if we're removing
|
||||
# a field from the module's field_data_cache, we should also
|
||||
# remove it from its _dirty_fields
|
||||
# pylint: disable=protected-access
|
||||
if field in self._dirty_fields:
|
||||
del self._dirty_fields[field]
|
||||
|
||||
# Set the new xmodule_runtime and field_data (which are user-specific)
|
||||
self.xmodule_runtime = xmodule_runtime
|
||||
|
||||
@@ -1360,23 +1360,44 @@ class TestRebindModule(TestSubmittingProblems):
|
||||
super(TestRebindModule, self).setUp()
|
||||
self.homework = self.add_graded_section_to_course('homework')
|
||||
self.lti = ItemFactory.create(category='lti', parent=self.homework)
|
||||
self.problem = ItemFactory.create(category='problem', parent=self.homework)
|
||||
self.user = UserFactory.create()
|
||||
self.anon_user = AnonymousUser()
|
||||
|
||||
def get_module_for_user(self, user):
|
||||
def get_module_for_user(self, user, item=None):
|
||||
"""Helper function to get useful module at self.location in self.course_id for user"""
|
||||
mock_request = MagicMock()
|
||||
mock_request.user = user
|
||||
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
|
||||
self.course.id, user, self.course, depth=2)
|
||||
|
||||
if item is None:
|
||||
item = self.lti
|
||||
|
||||
return render.get_module( # pylint: disable=protected-access
|
||||
user,
|
||||
mock_request,
|
||||
self.lti.location,
|
||||
item.location,
|
||||
field_data_cache,
|
||||
)._xmodule
|
||||
|
||||
def test_rebind_module_to_new_users(self):
|
||||
module = self.get_module_for_user(self.user, self.problem)
|
||||
|
||||
# Bind the module to another student, which will remove "correct_map"
|
||||
# from the module's _field_data_cache and _dirty_fields.
|
||||
user2 = UserFactory.create()
|
||||
module.descriptor.bind_for_student(module.system, user2.id)
|
||||
|
||||
# XBlock's save method assumes that if a field is in _dirty_fields,
|
||||
# then it's also in _field_data_cache. If this assumption
|
||||
# doesn't hold, then we get an error trying to bind this module
|
||||
# to a third student, since we've removed "correct_map" from
|
||||
# _field_data cache, but not _dirty_fields, when we bound
|
||||
# this module to the second student. (TNL-2640)
|
||||
user3 = UserFactory.create()
|
||||
module.descriptor.bind_for_student(module.system, user3.id)
|
||||
|
||||
def test_rebind_noauth_module_to_user_not_anonymous(self):
|
||||
"""
|
||||
Tests that an exception is thrown when rebind_noauth_module_to_user is run from a
|
||||
|
||||
@@ -7,6 +7,7 @@ http://stackoverflow.com/questions/10060069/safely-extract-zip-or-tar-using-pyth
|
||||
"""
|
||||
from os.path import abspath, realpath, dirname, join as joinpath
|
||||
from django.core.exceptions import SuspiciousOperation
|
||||
from django.conf import settings
|
||||
import logging
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -28,19 +29,23 @@ def _is_bad_path(path, base):
|
||||
|
||||
def _is_bad_link(info, base):
|
||||
"""
|
||||
Does the file sym- ord hard-link to files outside `base`?
|
||||
Does the file sym- or hard-link to files outside `base`?
|
||||
"""
|
||||
# Links are interpreted relative to the directory containing the link
|
||||
tip = resolved(joinpath(base, dirname(info.name)))
|
||||
return _is_bad_path(info.linkname, base=tip)
|
||||
|
||||
|
||||
def safemembers(members):
|
||||
def safemembers(members, base):
|
||||
"""
|
||||
Check that all elements of a tar file are safe.
|
||||
"""
|
||||
|
||||
base = resolved(".")
|
||||
base = resolved(base)
|
||||
|
||||
# check that we're not trying to import outside of the data_dir
|
||||
if not base.startswith(resolved(settings.DATA_DIR)):
|
||||
raise SuspiciousOperation("Attempted to import course outside of data dir")
|
||||
|
||||
for finfo in members:
|
||||
if _is_bad_path(finfo.name, base):
|
||||
@@ -61,8 +66,8 @@ def safemembers(members):
|
||||
return members
|
||||
|
||||
|
||||
def safetar_extractall(tarf, *args, **kwargs):
|
||||
def safetar_extractall(tar_file, path=".", members=None): # pylint: disable=unused-argument
|
||||
"""
|
||||
Safe version of `tarf.extractall()`.
|
||||
Safe version of `tar_file.extractall()`.
|
||||
"""
|
||||
return tarf.extractall(members=safemembers(tarf), *args, **kwargs)
|
||||
return tar_file.extractall(path, safemembers(tar_file, path))
|
||||
|
||||
Reference in New Issue
Block a user