Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2093,7 +2093,10 @@

################## BLOCKSTORE RELATED SETTINGS #########################
BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250'
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1'
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/'
Comment thread
bradenmacdonald marked this conversation as resolved.
Outdated
# Which of django's caches to use for storing anonymous user state for XBlocks
# in the blockstore-based XBlock runtime
XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are we calling it v2 runtime or blockstore runtime?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the v2 runtime can support other backends besides blockstore. But I use the terms somewhat interchangeably.


###################### LEARNER PORTAL ################################
LEARNER_PORTAL_URL_ROOT = 'https://learner-portal-localhost:18000'
Expand Down
14 changes: 14 additions & 0 deletions common/lib/xmodule/xmodule/capa_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,20 @@ def student_view(self, _context, show_detailed_errors=False):
shim_xmodule_js(fragment, 'Problem')
return fragment

def public_view(self, context):
"""
Return the view seen by users who aren't logged in or who aren't
enrolled in the course.
"""
if getattr(self.runtime, 'suppports_state_for_anonymous_users', False):
# The new XBlock runtime can generally support capa problems for users who aren't logged in, so show the
# normal student_view. To prevent anonymous users from viewing specific problems, adjust course policies
# and/or content groups.
return self.student_view(context)
else:
# Show a message that this content requires users to login/enroll.
return super(ProblemBlock, self).public_view(context)

def author_view(self, context):
"""
Renders the Studio preview view.
Expand Down
2 changes: 2 additions & 0 deletions common/lib/xmodule/xmodule/unit_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def student_view(self, context=None):
result.add_content('</div>')
return result

public_view = student_view

def index_dictionary(self):
"""
Return dictionary prepared with module content and type for indexing, so
Expand Down
11 changes: 5 additions & 6 deletions common/lib/xmodule/xmodule/video_module/video_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ def public_view(self, context):
"""
Returns a fragment that contains the html for the public view
"""
if getattr(self.runtime, 'suppports_state_for_anonymous_users', False):
# The new runtime can support anonymous users as fully as regular users:
return self.student_view(context)
return Fragment(self.get_html(view=PUBLIC_VIEW))

def get_html(self, view=STUDENT_VIEW):
Expand Down Expand Up @@ -610,12 +613,8 @@ def parse_xml_new_runtime(cls, node, runtime, keys):
field_data = cls.parse_video_xml(node)
for key, val in field_data.items():
setattr(video_block, key, cls.fields[key].from_json(val))
# Update VAL with info extracted from `xml_object`
video_block.edx_video_id = video_block.import_video_info_into_val(
node,
runtime.resources_fs,
keys.usage_id.context_key,
)
# Don't use VAL in the new runtime:
video_block.edx_video_id = None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a seemingly unrelated change, but I found that the public_view of Video XBlocks wasn't working for anonymous users when this VAL code was used, and we have also been sometimes seeing 500 errors on LX when the edx_video_id is present. Also, this was a bit problematic because it makes a lot of blocking API calls to VAL during XML parsing. So for those three reasons, I think that completely omitting this "update VAL" step when parsing video XML in the new runtime will work much better.

return video_block

@classmethod
Expand Down
5 changes: 4 additions & 1 deletion lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3834,7 +3834,10 @@ def _make_locale_paths(settings): # pylint: disable=missing-docstring

########################## BLOCKSTORE #####################################
BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250'
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1'
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/'
# Which of django's caches to use for storing anonymous user state for XBlocks
# in the blockstore-based XBlock runtime
XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default'

########################## LEARNER PORTAL ##############################
LEARNER_PORTAL_URL_ROOT = 'https://learner-portal-localhost:18000'
Expand Down
1 change: 1 addition & 0 deletions lms/envs/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1')
BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/")
BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key')
XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'blockstore' # This must be set to a working cache for the tests to pass

# Dummy secret key for dev
SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
Expand Down
113 changes: 94 additions & 19 deletions openedx/core/djangoapps/content_libraries/tests/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
"""
from __future__ import absolute_import, division, print_function, unicode_literals
import json
import unittest

from completion.test_utils import CompletionWaffleTestMixin
from django.test import TestCase
from django.test import TestCase, override_settings
from organizations.models import Organization
from rest_framework.test import APIClient
from xblock.core import XBlock, Scope
from xblock import fields
from xblock.core import XBlock

from lms.djangoapps.courseware.model_data import get_score
from openedx.core.djangoapps.content_libraries import api as library_api
Expand All @@ -20,27 +18,14 @@
URL_BLOCK_RENDER_VIEW,
URL_BLOCK_GET_HANDLER_URL,
)
from openedx.core.djangoapps.content_libraries.tests.user_state_block import UserStateTestBlock
from openedx.core.djangoapps.xblock import api as xblock_api
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.lib import blockstore_api
from student.tests.factories import UserFactory
from xmodule.unit_block import UnitBlock


class UserStateTestBlock(XBlock):
"""
Block for testing variously scoped XBlock fields.
"""
BLOCK_TYPE = "user-state-test"

display_name = fields.String(scope=Scope.content, name='User State Test Block')
# User-specific fields:
user_str = fields.String(scope=Scope.user_state, default='default value') # This usage, one user
uss_str = fields.String(scope=Scope.user_state_summary, default='default value') # This usage, all users
pref_str = fields.String(scope=Scope.preferences, default='default value') # Block type, one user
user_info_str = fields.String(scope=Scope.user_info, default='default value') # All blocks, one user


class ContentLibraryContentTestMixin(object):
"""
Mixin for content library tests that creates two students and a library.
Expand Down Expand Up @@ -69,6 +54,8 @@ def setUpClass(cls):


@requires_blockstore
# EphemeralKeyValueStore requires a working cache, and the default test cache doesn't work:
@override_settings(XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE='blockstore')
class ContentLibraryRuntimeTest(ContentLibraryContentTestMixin, TestCase):
"""
Basic tests of the Blockstore-based XBlock runtime using XBlocks in a
Expand Down Expand Up @@ -168,13 +155,101 @@ def test_modify_state_directly(self):
self.assertEqual(block1_bob.pref_str, 'default value')
self.assertEqual(block1_bob.user_info_str, 'default value')

@XBlock.register_temp_plugin(UserStateTestBlock, UserStateTestBlock.BLOCK_TYPE)
def test_state_for_anonymous_users(self):
"""
Test that anonymous users can interact with XBlocks and get/set their
state via handlers.
"""
# Create two XBlocks, block1 and block2
block1_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3-1")
block1_usage_key = block1_metadata.usage_key
block2_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3-2")
block2_usage_key = block2_metadata.usage_key
library_api.publish_changes(self.library.key)
# Create two clients (anonymous user's browsers)
client1 = APIClient()
client2 = APIClient()

def call_handler(client, block_key, handler_name, method, data=None):
""" Call an XBlock handler """
url_result = client.get(URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name))
url = url_result.data["handler_url"]
data_json = json.dumps(data) if data else None
response = getattr(client, method)(url, data_json, content_type="application/json")
self.assertEqual(response.status_code, 200)
return response.json()

# Now client1 sets all the fields via a handler:
call_handler(client1, block1_usage_key, "set_user_state", "post", {
"user_str": "1 was here",
"uss_str": "1 was here (USS)",
"pref_str": "1 was here (prefs)",
"user_info_str": "1 was here (user info)",
})

# Now load it back and expect the same data:
data = call_handler(client1, block1_usage_key, "get_user_state", "get")
self.assertEqual(data["user_str"], "1 was here")
self.assertEqual(data["uss_str"], "1 was here (USS)")
self.assertEqual(data["pref_str"], "1 was here (prefs)")
self.assertEqual(data["user_info_str"], "1 was here (user info)")

# Now load a different XBlock and expect only pref_str and user_info_str to be set:
data = call_handler(client1, block2_usage_key, "get_user_state", "get")
self.assertEqual(data["user_str"], "default value")
self.assertEqual(data["uss_str"], "default value")
self.assertEqual(data["pref_str"], "1 was here (prefs)")
self.assertEqual(data["user_info_str"], "1 was here (user info)")

# Now a different anonymous user loading the first block should see only the uss_str set:
data = call_handler(client2, block1_usage_key, "get_user_state", "get")
self.assertEqual(data["user_str"], "default value")
self.assertEqual(data["uss_str"], "1 was here (USS)")
self.assertEqual(data["pref_str"], "default value")
self.assertEqual(data["user_info_str"], "default value")

# The "user state summary" should not be shared between registered and anonymous users:
client_registered = APIClient()
client_registered.login(username=self.student_a.username, password='edx')
data = call_handler(client_registered, block1_usage_key, "get_user_state", "get")
self.assertEqual(data["user_str"], "default value")
self.assertEqual(data["uss_str"], "default value")
self.assertEqual(data["pref_str"], "default value")
self.assertEqual(data["user_info_str"], "default value")

def test_views_for_anonymous_users(self):
"""
Test that anonymous users can view XBlock's 'public_view' but not other
views
"""
# Create an XBlock
block_metadata = library_api.create_library_block(self.library.key, "html", "html1")
block_usage_key = block_metadata.usage_key
library_api.set_library_block_olx(block_usage_key, "<html>Hello world</html>")
library_api.publish_changes(self.library.key)

anon_client = APIClient()
# View the public_view:
public_view_result = anon_client.get(
URL_BLOCK_RENDER_VIEW.format(block_key=block_usage_key, view_name='public_view'),
)
self.assertEqual(public_view_result.status_code, 200)
self.assertIn("Hello world", public_view_result.data["content"])

# Try to view the student_view:
public_view_result = anon_client.get(
URL_BLOCK_RENDER_VIEW.format(block_key=block_usage_key, view_name='student_view'),
)
self.assertEqual(public_view_result.status_code, 403)

@XBlock.register_temp_plugin(UserStateTestBlock, UserStateTestBlock.BLOCK_TYPE)
def test_independent_instances(self):
"""
Test that independent instances of the same block don't share field data
until .save() and re-loading, even when they're using the same runtime.
"""
block_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3")
block_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b4")
block_usage_key = block_metadata.usage_key
library_api.publish_changes(self.library.key)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# -*- coding: utf-8 -*-
"""
Block for testing variously scoped XBlock fields.
"""
from __future__ import absolute_import, division, print_function, unicode_literals
import json

from webob import Response
from xblock.core import XBlock, Scope
from xblock import fields


class UserStateTestBlock(XBlock):
"""
Block for testing variously scoped XBlock fields.
"""
BLOCK_TYPE = "user-state-test"
has_score = False

display_name = fields.String(scope=Scope.content, name='User State Test Block')
# User-specific fields:
user_str = fields.String(scope=Scope.user_state, default='default value') # This usage, one user
uss_str = fields.String(scope=Scope.user_state_summary, default='default value') # This usage, all users
pref_str = fields.String(scope=Scope.preferences, default='default value') # Block type, one user
user_info_str = fields.String(scope=Scope.user_info, default='default value') # All blocks, one user

@XBlock.json_handler
def set_user_state(self, data, suffix): # pylint: disable=unused-argument
"""
Set the user-scoped fields
"""
self.user_str = data["user_str"]
self.uss_str = data["uss_str"]
self.pref_str = data["pref_str"]
self.user_info_str = data["user_info_str"]
return {}

@XBlock.handler
def get_user_state(self, request, suffix=None): # pylint: disable=unused-argument
"""
Get the various user-scoped fields of this XBlock.
"""
return Response(
json.dumps({
"user_str": self.user_str,
"uss_str": self.uss_str,
"pref_str": self.pref_str,
"user_info_str": self.user_info_str,
}),
content_type='application/json',
charset='UTF-8',
)
44 changes: 21 additions & 23 deletions openedx/core/djangoapps/xblock/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from openedx.core.djangoapps.xblock.runtime.blockstore_runtime import BlockstoreXBlockRuntime, xml_for_definition
from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntimeSystem
from openedx.core.djangolib.blockstore_cache import BundleCache
from .utils import get_secure_token_for_xblock_handler
from .utils import get_secure_token_for_xblock_handler, get_xblock_id_for_anonymous_user

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -160,11 +160,9 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen
"""
Get the HTML, JS, and CSS needed to render the given XBlock view.

The difference between this method and calling
The only difference between this method and calling
load_block().render(view_name)
is that this method will automatically save any changes to field data that
resulted from rendering the view. If you don't want that, call .render()
directly.
is that this method can fall back from 'author_view' to 'student_view'

Returns a Fragment.
"""
Expand All @@ -179,14 +177,10 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen
else:
raise

# TODO: save any changed user state fields
# TODO: if the view is anything other than student_view and we're not in the LMS, save any changed
# content/settings/children fields.

return fragment


def get_handler_url(usage_key, handler_name, user_id):
def get_handler_url(usage_key, handler_name, user):
"""
A method for getting the URL to any XBlock handler. The URL must be usable
without any authentication (no cookie, no OAuth/JWT), and may expire. (So
Expand All @@ -202,26 +196,30 @@ def get_handler_url(usage_key, handler_name, user_id):
Params:
usage_key - Usage Key (Opaque Key object or string)
handler_name - Name of the handler or a dummy name like 'any_handler'
user_id - User ID or XBlockRuntimeSystem.ANONYMOUS_USER
user - Django User (registered or anonymous)

This view does not check/care if the XBlock actually exists.
"""
usage_key_str = six.text_type(usage_key)
site_root_url = get_xblock_app_config().get_site_root_url()
if user_id is None:
if not user:
raise TypeError("Cannot get handler URLs without specifying a specific user ID.")
elif user_id == XBlockRuntimeSystem.ANONYMOUS_USER:
raise NotImplementedError("handler links for anonymous users are not yet implemented") # TODO: implement
elif user.is_authenticated:
user_id = user.id
elif user.is_anonymous:
user_id = get_xblock_id_for_anonymous_user(user)
else:
# Normal case: generate a token-secured URL for this handler, specific
# to this user and this XBlock.
secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str)
path = reverse('xblock_api:xblock_handler', kwargs={
'usage_key_str': usage_key_str,
'user_id': user_id,
'secure_token': secure_token,
'handler_name': handler_name,
})
raise ValueError("Invalid user value")
# Now generate a token-secured URL for this handler, specific to this user
# and this XBlock:
secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str)
# Now generate the URL to that handler:
path = reverse('xblock_api:xblock_handler', kwargs={
'usage_key_str': usage_key_str,
'user_id': user_id,
'secure_token': secure_token,
'handler_name': handler_name,
})
# We must return an absolute URL. We can't just use
# rest_framework.reverse.reverse to get the absolute URL because this method
# can be called by the XBlock from python as well and in that case we don't
Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/xblock/rest_api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
url(r'^handler_url/(?P<handler_name>[\w\-]+)/$', views.get_handler_url),
# call one of this block's handlers
url(
r'^handler/(?P<user_id>\d+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
r'^handler/(?P<user_id>\w+)-(?P<secure_token>\w+)/(?P<handler_name>[\w\-]+)/(?P<suffix>.+)?$',
views.xblock_handler,
name='xblock_handler',
),
Expand Down
Loading