Commit 4304c66c by Usman Khalid Committed by muzaffaryousaf

Performance optimizations and cache to keep the bookmarks info updated.

The cache uses the bookmarks.XBlockCache model.

TNL-1945
parent dbb52326
...@@ -803,6 +803,9 @@ INSTALLED_APPS = ( ...@@ -803,6 +803,9 @@ INSTALLED_APPS = (
# edX Proctoring # edX Proctoring
'edx_proctoring', 'edx_proctoring',
# Bookmarks
'openedx.core.djangoapps.bookmarks',
# programs support # programs support
'openedx.core.djangoapps.programs', 'openedx.core.djangoapps.programs',
......
...@@ -6,7 +6,7 @@ from .exceptions import (ItemNotFoundError, NoPathToItem) ...@@ -6,7 +6,7 @@ from .exceptions import (ItemNotFoundError, NoPathToItem)
LOGGER = getLogger(__name__) LOGGER = getLogger(__name__)
def path_to_location(modulestore, usage_key): def path_to_location(modulestore, usage_key, full_path=False):
''' '''
Try to find a course_id/chapter/section[/position] path to location in Try to find a course_id/chapter/section[/position] path to location in
modulestore. The courseware insists that the first level in the course is modulestore. The courseware insists that the first level in the course is
...@@ -15,6 +15,7 @@ def path_to_location(modulestore, usage_key): ...@@ -15,6 +15,7 @@ def path_to_location(modulestore, usage_key):
Args: Args:
modulestore: which store holds the relevant objects modulestore: which store holds the relevant objects
usage_key: :class:`UsageKey` the id of the location to which to generate the path usage_key: :class:`UsageKey` the id of the location to which to generate the path
full_path: :class:`Bool` if True, return the full path to location. Default is False.
Raises Raises
ItemNotFoundError if the location doesn't exist. ItemNotFoundError if the location doesn't exist.
...@@ -81,6 +82,9 @@ def path_to_location(modulestore, usage_key): ...@@ -81,6 +82,9 @@ def path_to_location(modulestore, usage_key):
if path is None: if path is None:
raise NoPathToItem(usage_key) raise NoPathToItem(usage_key)
if full_path:
return path
n = len(path) n = len(path)
course_id = path[0].course_key course_id = path[0].course_key
# pull out the location names # pull out the location names
......
...@@ -18,10 +18,12 @@ from openedx.core.lib.tempdir import mkdtemp_clean ...@@ -18,10 +18,12 @@ from openedx.core.lib.tempdir import mkdtemp_clean
from xmodule.contentstore.django import _CONTENTSTORE from xmodule.contentstore.django import _CONTENTSTORE
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.django import modulestore, clear_existing_modulestores, SignalHandler
from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK
from openedx.core.djangoapps.bookmarks.signals import trigger_update_xblocks_cache_task
class StoreConstructors(object): class StoreConstructors(object):
"""Enumeration of store constructor types.""" """Enumeration of store constructor types."""
...@@ -405,6 +407,8 @@ class ModuleStoreTestCase(TestCase): ...@@ -405,6 +407,8 @@ class ModuleStoreTestCase(TestCase):
super(ModuleStoreTestCase, self).setUp() super(ModuleStoreTestCase, self).setUp()
SignalHandler.course_published.disconnect(trigger_update_xblocks_cache_task)
self.store = modulestore() self.store = modulestore()
uname = 'testuser' uname = 'testuser'
......
"""
Models for Bookmarks.
"""
from django.contrib.auth.models import User
from django.db import models
from jsonfield.fields import JSONField
from model_utils.models import TimeStampedModel
from xmodule.modulestore.django import modulestore
from xmodule_django.models import CourseKeyField, LocationKeyField
class Bookmark(TimeStampedModel):
"""
Bookmarks model.
"""
user = models.ForeignKey(User, db_index=True)
course_key = CourseKeyField(max_length=255, db_index=True)
usage_key = LocationKeyField(max_length=255, db_index=True)
display_name = models.CharField(max_length=255, default='', help_text='Display name of block')
path = JSONField(help_text='Path in course tree to the block')
@classmethod
def create(cls, bookmark_data):
"""
Create a Bookmark object.
Arguments:
bookmark_data (dict): The data to create the object with.
Returns:
A Bookmark object.
Raises:
ItemNotFoundError: If no block exists for the usage_key.
"""
usage_key = bookmark_data.pop('usage_key')
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
block = modulestore().get_item(usage_key)
bookmark_data['course_key'] = usage_key.course_key
bookmark_data['display_name'] = block.display_name
bookmark_data['path'] = cls.get_path(block)
user = bookmark_data.pop('user')
return cls.objects.get_or_create(usage_key=usage_key, user=user, defaults=bookmark_data)
@staticmethod
def get_path(block):
"""
Returns data for the path to the block in the course tree.
Arguments:
block (XBlock): The block whose path is required.
Returns:
list of dicts of the form {'usage_id': <usage_id>, 'display_name': <display_name>}.
"""
parent = block.get_parent()
parents_data = []
while parent is not None and parent.location.block_type not in ['course']:
parents_data.append({"display_name": parent.display_name, "usage_id": unicode(parent.location)})
parent = parent.get_parent()
parents_data.reverse()
return parents_data
@property
def resource_id(self):
"""
Return the resource id: {username,usage_id}.
"""
return "{0},{1}".format(self.user.username, self.usage_key) # pylint: disable=no-member
"""
Tests for bookmarks api.
"""
from django.core.exceptions import ObjectDoesNotExist
from opaque_keys.edx.keys import UsageKey
from student.tests.factories import UserFactory
from util.testing import EventTestMixin
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from .factories import BookmarkFactory
from .. import api, DEFAULT_FIELDS, OPTIONAL_FIELDS
from ..models import Bookmark
class BookmarkApiEventTestMixin(EventTestMixin):
""" Mixin for verifying that bookmark api events were emitted during a test. """
def setUp(self): # pylint: disable=arguments-differ
super(BookmarkApiEventTestMixin, self).setUp('lms.djangoapps.bookmarks.api.tracker')
def assert_bookmark_event_emitted(self, event_name, course_id, bookmark_id, usage_key):
""" Assert that an event has been emitted. """
self.assert_event_emitted(
event_name,
course_id=course_id,
bookmark_id=bookmark_id,
component_type=usage_key.category,
component_usage_id=unicode(usage_key),
)
class BookmarksAPITests(BookmarkApiEventTestMixin, ModuleStoreTestCase):
"""
These tests cover the parts of the API methods.
"""
def setUp(self):
super(BookmarksAPITests, self).setUp()
self.user = UserFactory.create(password='test')
self.other_user = UserFactory.create(password='test')
self.course = CourseFactory.create(display_name='An Introduction to API Testing')
self.course_id = unicode(self.course.id)
self.chapter = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 1'
)
self.sequential = ItemFactory.create(
parent_location=self.chapter.location, category='sequential', display_name='Lesson 1'
)
self.vertical = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 1'
)
self.vertical_1 = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 1.1'
)
self.bookmark = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=self.vertical.location,
display_name=self.vertical.display_name
)
self.course_2 = CourseFactory.create(display_name='An Introduction to API Testing 2')
self.chapter_2 = ItemFactory.create(
parent_location=self.course_2.location, category='chapter', display_name='Week 2'
)
self.sequential_2 = ItemFactory.create(
parent_location=self.chapter_2.location, category='sequential', display_name='Lesson 2'
)
self.vertical_2 = ItemFactory.create(
parent_location=self.sequential_2.location, category='vertical', display_name='Subsection 2'
)
self.bookmark_2 = BookmarkFactory.create(
user=self.user,
course_key=self.course_2.id,
usage_key=self.vertical_2.location,
display_name=self.vertical_2.display_name
)
self.all_fields = DEFAULT_FIELDS + OPTIONAL_FIELDS
self.reset_tracker()
def assert_bookmark_response(self, response_data, bookmark, optional_fields=False):
"""
Determines if the given response data (dict) matches the given bookmark.
"""
self.assertEqual(response_data['id'], '%s,%s' % (self.user.username, unicode(bookmark.usage_key)))
self.assertEqual(response_data['course_id'], unicode(bookmark.course_key))
self.assertEqual(response_data['usage_id'], unicode(bookmark.usage_key))
self.assertEqual(response_data['block_type'], unicode(bookmark.usage_key.block_type))
self.assertIsNotNone(response_data['created'])
if optional_fields:
self.assertEqual(response_data['display_name'], bookmark.display_name)
self.assertEqual(response_data['path'], bookmark.path)
def test_get_bookmark(self):
"""
Verifies that get_bookmark returns data as expected.
"""
bookmark_data = api.get_bookmark(user=self.user, usage_key=self.vertical.location)
self.assert_bookmark_response(bookmark_data, self.bookmark)
# With Optional fields.
bookmark_data = api.get_bookmark(
user=self.user,
usage_key=self.vertical.location,
fields=self.all_fields
)
self.assert_bookmark_response(bookmark_data, self.bookmark, optional_fields=True)
def test_get_bookmark_raises_error(self):
"""
Verifies that get_bookmark raises error as expected.
"""
with self.assertRaises(ObjectDoesNotExist):
api.get_bookmark(user=self.other_user, usage_key=self.vertical.location)
def test_get_bookmarks(self):
"""
Verifies that get_bookmarks returns data as expected.
"""
# Without course key.
bookmarks_data = api.get_bookmarks(user=self.user)
self.assertEqual(len(bookmarks_data), 2)
# Assert them in ordered manner.
self.assert_bookmark_response(bookmarks_data[0], self.bookmark_2)
self.assert_bookmark_response(bookmarks_data[1], self.bookmark)
# With course key.
bookmarks_data = api.get_bookmarks(user=self.user, course_key=self.course.id)
self.assertEqual(len(bookmarks_data), 1)
self.assert_bookmark_response(bookmarks_data[0], self.bookmark)
# With optional fields.
bookmarks_data = api.get_bookmarks(user=self.user, course_key=self.course.id, fields=self.all_fields)
self.assertEqual(len(bookmarks_data), 1)
self.assert_bookmark_response(bookmarks_data[0], self.bookmark, optional_fields=True)
# Without Serialized.
bookmarks = api.get_bookmarks(user=self.user, course_key=self.course.id, serialized=False)
self.assertEqual(len(bookmarks), 1)
self.assertTrue(bookmarks.model is Bookmark) # pylint: disable=no-member
self.assertEqual(bookmarks[0], self.bookmark)
def test_create_bookmark(self):
"""
Verifies that create_bookmark create & returns data as expected.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 1)
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.added',
self.course_id,
bookmark_data['id'],
self.vertical_1.location
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
def test_create_bookmark_do_not_create_duplicates(self):
"""
Verifies that create_bookmark do not create duplicate bookmarks.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 1)
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.added',
self.course_id,
bookmark_data['id'],
self.vertical_1.location
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
self.reset_tracker()
bookmark_data_2 = api.create_bookmark(user=self.user, usage_key=self.vertical_1.location)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
self.assertEqual(bookmark_data, bookmark_data_2)
self.assert_no_events_were_emitted()
def test_create_bookmark_raises_error(self):
"""
Verifies that create_bookmark raises error as expected.
"""
with self.assertRaises(ItemNotFoundError):
api.create_bookmark(user=self.user, usage_key=UsageKey.from_string('i4x://brb/100/html/340ef1771a0940'))
self.assert_no_events_were_emitted()
def test_delete_bookmark(self):
"""
Verifies that delete_bookmark removes bookmark as expected.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user)), 2)
api.delete_bookmark(user=self.user, usage_key=self.vertical.location)
self.assert_bookmark_event_emitted(
'edx.bookmark.removed',
self.course_id,
self.bookmark.resource_id,
self.vertical.location
)
bookmarks_data = api.get_bookmarks(user=self.user)
self.assertEqual(len(bookmarks_data), 1)
self.assertNotEqual(unicode(self.vertical.location), bookmarks_data[0]['usage_id'])
def test_delete_bookmark_raises_error(self):
"""
Verifies that delete_bookmark raises error as expected.
"""
with self.assertRaises(ObjectDoesNotExist):
api.delete_bookmark(user=self.other_user, usage_key=self.vertical.location)
self.assert_no_events_were_emitted()
"""
Tests for Bookmarks models.
"""
from bookmarks.models import Bookmark
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
class BookmarkModelTest(ModuleStoreTestCase):
"""
Test the Bookmark model.
"""
def setUp(self):
super(BookmarkModelTest, self).setUp()
self.user = UserFactory.create(password='test')
self.course = CourseFactory.create(display_name='An Introduction to API Testing')
self.course_id = unicode(self.course.id)
self.chapter = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 1'
)
self.sequential = ItemFactory.create(
parent_location=self.chapter.location, category='sequential', display_name='Lesson 1'
)
self.vertical = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 1'
)
self.vertical_2 = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 2'
)
self.path = [
{'display_name': self.chapter.display_name, 'usage_id': unicode(self.chapter.location)},
{'display_name': self.sequential.display_name, 'usage_id': unicode(self.sequential.location)}
]
def get_bookmark_data(self, block):
"""
Returns bookmark data for testing.
"""
return {
'user': self.user,
'course_key': self.course.id,
'usage_key': block.location,
'display_name': block.display_name,
}
def assert_valid_bookmark(self, bookmark_object, bookmark_data):
"""
Check if the given data matches the specified bookmark.
"""
self.assertEqual(bookmark_object.user, self.user)
self.assertEqual(bookmark_object.course_key, bookmark_data['course_key'])
self.assertEqual(bookmark_object.usage_key, self.vertical.location)
self.assertEqual(bookmark_object.display_name, bookmark_data['display_name'])
self.assertEqual(bookmark_object.path, self.path)
self.assertIsNotNone(bookmark_object.created)
def test_create_bookmark_success(self):
"""
Tests creation of bookmark.
"""
bookmark_data = self.get_bookmark_data(self.vertical)
bookmark_object, __ = Bookmark.create(bookmark_data)
self.assert_valid_bookmark(bookmark_object, bookmark_data)
def test_get_path(self):
"""
Tests creation of path with given block.
"""
path_object = Bookmark.get_path(block=self.vertical)
self.assertEqual(path_object, self.path)
def test_get_path_with_given_chapter_block(self):
"""
Tests path for chapter level block.
"""
path_object = Bookmark.get_path(block=self.chapter)
self.assertEqual(len(path_object), 0)
def test_get_path_with_given_sequential_block(self):
"""
Tests path for sequential level block.
"""
path_object = Bookmark.get_path(block=self.sequential)
self.assertEqual(len(path_object), 1)
self.assertEqual(path_object[0], self.path[0])
def test_get_path_returns_empty_list_for_unreachable_parent(self):
"""
Tests get_path returns empty list if block has no parent.
"""
path = Bookmark.get_path(block=self.course)
self.assertEqual(path, [])
"""
Tests for bookmark services.
"""
from opaque_keys.edx.keys import UsageKey
from .factories import BookmarkFactory
from ..services import BookmarksService
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
class BookmarksAPITests(ModuleStoreTestCase):
"""
Tests the Bookmarks service.
"""
def setUp(self):
super(BookmarksAPITests, self).setUp()
self.user = UserFactory.create(password='test')
self.other_user = UserFactory.create(password='test')
self.course = CourseFactory.create(display_name='An Introduction to API Testing')
self.course_id = unicode(self.course.id)
self.chapter = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 1'
)
self.sequential = ItemFactory.create(
parent_location=self.chapter.location, category='sequential', display_name='Lesson 1'
)
self.vertical = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 1'
)
self.vertical_1 = ItemFactory.create(
parent_location=self.sequential.location, category='vertical', display_name='Subsection 1.1'
)
self.bookmark = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=self.vertical.location,
display_name=self.vertical.display_name
)
self.bookmark_service = BookmarksService(user=self.user)
def assert_bookmark_response(self, response_data, bookmark):
"""
Determines if the given response data (dict) matches the specified bookmark.
"""
self.assertEqual(response_data['id'], '%s,%s' % (self.user.username, unicode(bookmark.usage_key)))
self.assertEqual(response_data['course_id'], unicode(bookmark.course_key))
self.assertEqual(response_data['usage_id'], unicode(bookmark.usage_key))
self.assertEqual(response_data['block_type'], unicode(bookmark.usage_key.block_type))
self.assertIsNotNone(response_data['created'])
self.assertEqual(response_data['display_name'], bookmark.display_name)
self.assertEqual(response_data['path'], bookmark.path)
def test_get_bookmarks(self):
"""
Verifies get_bookmarks returns data as expected.
"""
bookmarks_data = self.bookmark_service.bookmarks(course_key=self.course.id)
self.assertEqual(len(bookmarks_data), 1)
self.assert_bookmark_response(bookmarks_data[0], self.bookmark)
def test_is_bookmarked(self):
"""
Verifies is_bookmarked returns Bool as expected.
"""
self.assertTrue(self.bookmark_service.is_bookmarked(usage_key=self.vertical.location))
self.assertFalse(self.bookmark_service.is_bookmarked(usage_key=self.vertical_1.location))
# Get bookmark that does not exist.
bookmark_service = BookmarksService(self.other_user)
self.assertFalse(bookmark_service.is_bookmarked(usage_key=self.vertical.location))
def test_set_bookmarked(self):
"""
Verifies set_bookmarked returns Bool as expected.
"""
# Assert False for item that does not exist.
self.assertFalse(
self.bookmark_service.set_bookmarked(usage_key=UsageKey.from_string("i4x://ed/ed/ed/interactive"))
)
self.assertTrue(self.bookmark_service.set_bookmarked(usage_key=self.vertical_1.location))
def test_unset_bookmarked(self):
"""
Verifies unset_bookmarked returns Bool as expected.
"""
self.assertFalse(
self.bookmark_service.unset_bookmarked(usage_key=UsageKey.from_string("i4x://ed/ed/ed/interactive"))
)
self.assertTrue(self.bookmark_service.unset_bookmarked(usage_key=self.vertical.location))
...@@ -42,13 +42,13 @@ from courseware.entrance_exams import ( ...@@ -42,13 +42,13 @@ from courseware.entrance_exams import (
) )
from edxmako.shortcuts import render_to_string from edxmako.shortcuts import render_to_string
from eventtracking import tracker from eventtracking import tracker
from lms.djangoapps.bookmarks.services import BookmarksService
from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.field_data import LmsFieldData
from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, unquote_slashes, quote_slashes
from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.keys import UsageKey, CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from openedx.core.djangoapps.bookmarks.services import BookmarksService
from openedx.core.lib.xblock_utils import ( from openedx.core.lib.xblock_utils import (
replace_course_urls, replace_course_urls,
replace_jump_to_id_urls, replace_jump_to_id_urls,
......
...@@ -1909,6 +1909,7 @@ INSTALLED_APPS = ( ...@@ -1909,6 +1909,7 @@ INSTALLED_APPS = (
'xblock_django', 'xblock_django',
# Bookmarks # Bookmarks
'openedx.core.djangoapps.bookmarks',
'bookmarks', 'bookmarks',
# programs support # programs support
......
...@@ -90,7 +90,7 @@ urlpatterns = ( ...@@ -90,7 +90,7 @@ urlpatterns = (
url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')), url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
# Bookmarks API endpoints # Bookmarks API endpoints
url(r'^api/bookmarks/', include('bookmarks.urls')), url(r'^api/bookmarks/', include('openedx.core.djangoapps.bookmarks.urls')),
# Profile Images API endpoints # Profile Images API endpoints
url(r'^api/profile_images/', include('openedx.core.djangoapps.profile_images.urls')), url(r'^api/profile_images/', include('openedx.core.djangoapps.profile_images.urls')),
......
""" """
Bookmarks module. Bookmarks module.
""" """
from collections import namedtuple
DEFAULT_FIELDS = [ DEFAULT_FIELDS = [
'id', 'id',
...@@ -14,3 +16,5 @@ OPTIONAL_FIELDS = [ ...@@ -14,3 +16,5 @@ OPTIONAL_FIELDS = [
'display_name', 'display_name',
'path', 'path',
] ]
PathItem = namedtuple('PathItem', ['usage_key', 'display_name'])
...@@ -22,7 +22,14 @@ def get_bookmark(user, usage_key, fields=None): ...@@ -22,7 +22,14 @@ def get_bookmark(user, usage_key, fields=None):
Raises: Raises:
ObjectDoesNotExist: If a bookmark with the parameters does not exist. ObjectDoesNotExist: If a bookmark with the parameters does not exist.
""" """
bookmark = Bookmark.objects.get(user=user, usage_key=usage_key) bookmarks_queryset = Bookmark.objects
if len(set(fields or []) & set(OPTIONAL_FIELDS)) > 0:
bookmarks_queryset = bookmarks_queryset.select_related('user', 'xblock_cache')
else:
bookmarks_queryset = bookmarks_queryset.select_related('user')
bookmark = bookmarks_queryset.get(user=user, usage_key=usage_key)
return BookmarkSerializer(bookmark, context={'fields': fields}).data return BookmarkSerializer(bookmark, context={'fields': fields}).data
...@@ -46,6 +53,11 @@ def get_bookmarks(user, course_key=None, fields=None, serialized=True): ...@@ -46,6 +53,11 @@ def get_bookmarks(user, course_key=None, fields=None, serialized=True):
if course_key: if course_key:
bookmarks_queryset = bookmarks_queryset.filter(course_key=course_key) bookmarks_queryset = bookmarks_queryset.filter(course_key=course_key)
if len(set(fields or []) & set(OPTIONAL_FIELDS)) > 0:
bookmarks_queryset = bookmarks_queryset.select_related('user', 'xblock_cache')
else:
bookmarks_queryset = bookmarks_queryset.select_related('user')
bookmarks_queryset = bookmarks_queryset.order_by('-created') bookmarks_queryset = bookmarks_queryset.order_by('-created')
if serialized: if serialized:
......
# -*- coding: utf-8 -*-
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models
class Migration(SchemaMigration):
def forwards(self, orm):
# Adding model 'XBlockCache'
db.create_table('bookmarks_xblockcache', (
('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
('created', self.gf('model_utils.fields.AutoCreatedField')(default=datetime.datetime.now)),
('modified', self.gf('model_utils.fields.AutoLastModifiedField')(default=datetime.datetime.now)),
('course_key', self.gf('xmodule_django.models.CourseKeyField')(max_length=255, db_index=True)),
('usage_key', self.gf('xmodule_django.models.LocationKeyField')(unique=True, max_length=255, db_index=True)),
('display_name', self.gf('django.db.models.fields.CharField')(default='', max_length=255)),
('_paths', self.gf('jsonfield.fields.JSONField')(default=[], db_column='paths')),
))
db.send_create_signal('bookmarks', ['XBlockCache'])
# Deleting field 'Bookmark.display_name'
db.delete_column('bookmarks_bookmark', 'display_name')
# Deleting field 'Bookmark.path'
db.delete_column('bookmarks_bookmark', 'path')
# Adding field 'Bookmark._path'
db.add_column('bookmarks_bookmark', '_path',
self.gf('jsonfield.fields.JSONField')(default='', db_column='path'),
keep_default=False)
# Adding field 'Bookmark.xblock_cache'
db.add_column('bookmarks_bookmark', 'xblock_cache',
self.gf('django.db.models.fields.related.ForeignKey')(default=0, to=orm['bookmarks.XBlockCache']),
keep_default=False)
# Adding unique constraint on 'Bookmark', fields ['user', 'usage_key']
db.create_unique('bookmarks_bookmark', ['user_id', 'usage_key'])
def backwards(self, orm):
# Removing unique constraint on 'Bookmark', fields ['user', 'usage_key']
db.delete_unique('bookmarks_bookmark', ['user_id', 'usage_key'])
# Deleting model 'XBlockCache'
db.delete_table('bookmarks_xblockcache')
# Adding field 'Bookmark.display_name'
db.add_column('bookmarks_bookmark', 'display_name',
self.gf('django.db.models.fields.CharField')(default='', max_length=255),
keep_default=False)
# Adding field 'Bookmark.path'
db.add_column('bookmarks_bookmark', 'path',
self.gf('jsonfield.fields.JSONField')(default=''),
keep_default=False)
# Deleting field 'Bookmark._path'
db.delete_column('bookmarks_bookmark', 'path')
# Deleting field 'Bookmark.xblock_cache'
db.delete_column('bookmarks_bookmark', 'xblock_cache_id')
models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'bookmarks.bookmark': {
'Meta': {'unique_together': "(('user', 'usage_key'),)", 'object_name': 'Bookmark'},
'_path': ('jsonfield.fields.JSONField', [], {'db_column': "'path'"}),
'course_key': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
'usage_key': ('xmodule_django.models.LocationKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}),
'xblock_cache': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['bookmarks.XBlockCache']"})
},
'bookmarks.xblockcache': {
'Meta': {'object_name': 'XBlockCache'},
'_paths': ('jsonfield.fields.JSONField', [], {'default': '[]', 'db_column': "'paths'"}),
'course_key': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
'display_name': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
'usage_key': ('xmodule_django.models.LocationKeyField', [], {'unique': 'True', 'max_length': '255', 'db_index': 'True'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
}
}
complete_apps = ['bookmarks']
\ No newline at end of file
"""
Models for Bookmarks.
"""
import logging
from django.contrib.auth.models import User
from django.db import models
from jsonfield.fields import JSONField
from model_utils.models import TimeStampedModel
from opaque_keys.edx.keys import UsageKey
from xmodule.modulestore import search
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule_django.models import CourseKeyField, LocationKeyField
from . import PathItem
log = logging.getLogger(__name__)
def prepare_path_for_serialization(path):
"""
Return the data from a list of PathItems ready for serialization to json.
"""
return [(unicode(path_item.usage_key), path_item.display_name) for path_item in path]
def parse_path_data(path_data):
"""
Return a list of PathItems constructed from parsing path_data.
"""
path = []
for item in path_data:
usage_key = UsageKey.from_string(item[0])
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
path.append(PathItem(usage_key, item[1]))
return path
class Bookmark(TimeStampedModel):
"""
Bookmarks model.
"""
user = models.ForeignKey(User, db_index=True)
course_key = CourseKeyField(max_length=255, db_index=True)
usage_key = LocationKeyField(max_length=255, db_index=True)
_path = JSONField(db_column='path', help_text='Path in course tree to the block')
xblock_cache = models.ForeignKey('bookmarks.XBlockCache')
class Meta(object):
"""
Bookmark metadata.
"""
unique_together = ('user', 'usage_key')
@classmethod
def create(cls, data):
"""
Create a Bookmark object.
Arguments:
data (dict): The data to create the object with.
Returns:
A Bookmark object.
Raises:
ItemNotFoundError: If no block exists for the usage_key.
"""
data = dict(data)
usage_key = data.pop('usage_key')
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
with modulestore().bulk_operations(usage_key.course_key):
block = modulestore().get_item(usage_key)
xblock_cache = XBlockCache.create({
'usage_key': usage_key,
'display_name': block.display_name,
})
data['_path'] = prepare_path_for_serialization(Bookmark.updated_path(usage_key, xblock_cache))
data['course_key'] = usage_key.course_key
data['xblock_cache'] = xblock_cache
user = data.pop('user')
bookmark, created = cls.objects.get_or_create(usage_key=usage_key, user=user, defaults=data)
return bookmark, created
@property
def resource_id(self):
"""
Return the resource id: {username,usage_id}.
"""
return "{0},{1}".format(self.user.username, self.usage_key) # pylint: disable=no-member
@property
def display_name(self):
"""
Return the display_name from self.xblock_cache.
Returns:
String.
"""
return self.xblock_cache.display_name # pylint: disable=no-member
@property
def path(self):
"""
Return the path to the bookmark's block after checking self.xblock_cache.
Returns:
List of dicts.
"""
if self.modified < self.xblock_cache.modified: # pylint: disable=no-member
path = Bookmark.updated_path(self.usage_key, self.xblock_cache)
self._path = prepare_path_for_serialization(path)
self.save() # Always save so that self.modified is updated.
return path
return parse_path_data(self._path)
@staticmethod
def updated_path(usage_key, xblock_cache):
"""
Return the update-to-date path.
xblock_cache.paths is the list of all possible paths to a block
constructed by doing a DFS of the tree. However, in case of DAGS,
which section jump_to_id() takes the user to depends on the
modulestore. If xblock_cache.paths has only one item, we can
just use it. Otherwise, we use path_to_location() to get the path
jump_to_id() will take the user to.
"""
if xblock_cache.paths and len(xblock_cache.paths) == 1:
return xblock_cache.paths[0]
return Bookmark.get_path(usage_key)
@staticmethod
def get_path(usage_key):
"""
Returns data for the path to the block in the course graph.
Note: In case of multiple paths to the block from the course
root, this function returns a path arbitrarily but consistently,
depending on the modulestore. In the future, we may want to
extend it to check which of the paths, the user has access to
and return its data.
Arguments:
block (XBlock): The block whose path is required.
Returns:
list of PathItems
"""
with modulestore().bulk_operations(usage_key.course_key):
try:
path = search.path_to_location(modulestore(), usage_key, full_path=True)
except ItemNotFoundError:
log.error(u'Block with usage_key: %s not found.', usage_key)
return []
except NoPathToItem:
log.error(u'No path to block with usage_key: %s.', usage_key)
return []
path_data = []
for ancestor_usage_key in path:
if ancestor_usage_key != usage_key and ancestor_usage_key.block_type != 'course': # pylint: disable=no-member
try:
block = modulestore().get_item(ancestor_usage_key)
except ItemNotFoundError:
return [] # No valid path can be found.
path_data.append(
PathItem(usage_key=block.location, display_name=block.display_name)
)
return path_data
class XBlockCache(TimeStampedModel):
"""
XBlockCache model to store info about xblocks.
"""
course_key = CourseKeyField(max_length=255, db_index=True)
usage_key = LocationKeyField(max_length=255, db_index=True, unique=True)
display_name = models.CharField(max_length=255, default='')
_paths = JSONField(
db_column='paths', default=[], help_text='All paths in course tree to the corresponding block.'
)
@property
def paths(self):
"""
Return paths.
Returns:
list of list of PathItems.
"""
return [parse_path_data(path) for path in self._paths] if self._paths else self._paths
@paths.setter
def paths(self, value):
"""
Set paths.
Arguments:
value (list of list of PathItems): The list of paths to cache.
"""
self._paths = [prepare_path_for_serialization(path) for path in value] if value else value
@classmethod
def create(cls, data):
"""
Create an XBlockCache object.
Arguments:
data (dict): The data to create the object with.
Returns:
An XBlockCache object.
"""
data = dict(data)
usage_key = data.pop('usage_key')
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
data['course_key'] = usage_key.course_key
xblock_cache, created = cls.objects.get_or_create(usage_key=usage_key, defaults=data)
if not created:
new_display_name = data.get('display_name', xblock_cache.display_name)
if xblock_cache.display_name != new_display_name:
xblock_cache.display_name = new_display_name
xblock_cache.save()
return xblock_cache
...@@ -15,7 +15,8 @@ class BookmarkSerializer(serializers.ModelSerializer): ...@@ -15,7 +15,8 @@ class BookmarkSerializer(serializers.ModelSerializer):
course_id = serializers.Field(source='course_key') course_id = serializers.Field(source='course_key')
usage_id = serializers.Field(source='usage_key') usage_id = serializers.Field(source='usage_key')
block_type = serializers.Field(source='usage_key.block_type') block_type = serializers.Field(source='usage_key.block_type')
path = serializers.Field(source='path') display_name = serializers.Field(source='display_name')
path = serializers.SerializerMethodField('path_data')
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
# Don't pass the 'fields' arg up to the superclass # Don't pass the 'fields' arg up to the superclass
...@@ -44,3 +45,15 @@ class BookmarkSerializer(serializers.ModelSerializer): ...@@ -44,3 +45,15 @@ class BookmarkSerializer(serializers.ModelSerializer):
'path', 'path',
'created', 'created',
) )
def resource_id(self, bookmark):
"""
Return the REST resource id: {username,usage_id}.
"""
return "{0},{1}".format(bookmark.user.username, bookmark.usage_key)
def path_data(self, bookmark):
"""
Serialize and return the path data of the bookmark.
"""
return [path_item._asdict() for path_item in bookmark.path]
...@@ -5,22 +5,57 @@ import logging ...@@ -5,22 +5,57 @@ import logging
from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ObjectDoesNotExist
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from request_cache.middleware import RequestCache
from . import DEFAULT_FIELDS, OPTIONAL_FIELDS, api from . import DEFAULT_FIELDS, OPTIONAL_FIELDS, api
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
CACHE_KEY_TEMPLATE = u"bookmarks.list.{}.{}"
class BookmarksService(object): class BookmarksService(object):
""" """
A service that provides access to the bookmarks API. A service that provides access to the bookmarks API.
When bookmarks() or is_bookmarked() is called for the
first time, the service fetches and caches all the bookmarks
of the user for the relevant course. So multiple calls to
get bookmark status during a request (for, example when
rendering courseware and getting bookmarks status for search
results) will not cause repeated queries to the database.
""" """
def __init__(self, user, **kwargs): def __init__(self, user, **kwargs):
super(BookmarksService, self).__init__(**kwargs) super(BookmarksService, self).__init__(**kwargs)
self._user = user self._user = user
def _bookmarks_cache(self, course_key, fetch=False):
"""
Return the user's bookmarks cache for a particular course.
Arguments:
course_key (CourseKey): course_key of the course whose bookmarks cache should be returned.
fetch (Bool): if the bookmarks should be fetched and cached if they already aren't.
"""
if hasattr(modulestore(), 'fill_in_run'):
course_key = modulestore().fill_in_run(course_key)
if course_key.run is None:
return []
cache_key = CACHE_KEY_TEMPLATE.format(self._user.id, course_key)
bookmarks_cache = RequestCache.get_request_cache().data.get(cache_key, None)
if bookmarks_cache is None and fetch is True:
bookmarks_cache = api.get_bookmarks(
self._user, course_key=course_key, fields=DEFAULT_FIELDS + OPTIONAL_FIELDS
)
RequestCache.get_request_cache().data[cache_key] = bookmarks_cache
return bookmarks_cache
def bookmarks(self, course_key): def bookmarks(self, course_key):
""" """
Return a list of bookmarks for the course for the current user. Return a list of bookmarks for the course for the current user.
...@@ -31,7 +66,7 @@ class BookmarksService(object): ...@@ -31,7 +66,7 @@ class BookmarksService(object):
Returns: Returns:
list of dict: list of dict:
""" """
return api.get_bookmarks(self._user, course_key=course_key, fields=DEFAULT_FIELDS + OPTIONAL_FIELDS) return self._bookmarks_cache(course_key, fetch=True)
def is_bookmarked(self, usage_key): def is_bookmarked(self, usage_key):
""" """
...@@ -43,13 +78,13 @@ class BookmarksService(object): ...@@ -43,13 +78,13 @@ class BookmarksService(object):
Returns: Returns:
Bool Bool
""" """
try: usage_id = unicode(usage_key)
api.get_bookmark(user=self._user, usage_key=usage_key) bookmarks_cache = self._bookmarks_cache(usage_key.course_key, fetch=True)
except ObjectDoesNotExist: for bookmark in bookmarks_cache:
log.error(u'Bookmark with usage_id: %s does not exist.', usage_key) if bookmark['usage_id'] == usage_id:
return False return True
return True return False
def set_bookmarked(self, usage_key): def set_bookmarked(self, usage_key):
""" """
...@@ -62,11 +97,15 @@ class BookmarksService(object): ...@@ -62,11 +97,15 @@ class BookmarksService(object):
Bool indicating whether the bookmark was added. Bool indicating whether the bookmark was added.
""" """
try: try:
api.create_bookmark(user=self._user, usage_key=usage_key) bookmark = api.create_bookmark(user=self._user, usage_key=usage_key)
except ItemNotFoundError: except ItemNotFoundError:
log.error(u'Block with usage_id: %s not found.', usage_key) log.error(u'Block with usage_id: %s not found.', usage_key)
return False return False
bookmarks_cache = self._bookmarks_cache(usage_key.course_key)
if bookmarks_cache is not None:
bookmarks_cache.append(bookmark)
return True return True
def unset_bookmarked(self, usage_key): def unset_bookmarked(self, usage_key):
...@@ -85,4 +124,15 @@ class BookmarksService(object): ...@@ -85,4 +124,15 @@ class BookmarksService(object):
log.error(u'Bookmark with usage_id: %s does not exist.', usage_key) log.error(u'Bookmark with usage_id: %s does not exist.', usage_key)
return False return False
bookmarks_cache = self._bookmarks_cache(usage_key.course_key)
if bookmarks_cache is not None:
deleted_bookmark_index = None
usage_id = unicode(usage_key)
for index, bookmark in enumerate(bookmarks_cache):
if bookmark['usage_id'] == usage_id:
deleted_bookmark_index = index
break
if deleted_bookmark_index is not None:
bookmarks_cache.pop(deleted_bookmark_index)
return True return True
"""
Signals for bookmarks.
"""
from importlib import import_module
from django.dispatch.dispatcher import receiver
from xmodule.modulestore.django import SignalHandler
@receiver(SignalHandler.course_published)
def trigger_update_xblocks_cache_task(sender, course_key, **kwargs): # pylint: disable=invalid-name,unused-argument
"""
Trigger update_xblocks_cache() when course_published signal is fired.
"""
tasks = import_module('openedx.core.djangoapps.bookmarks.tasks') # Importing tasks early causes issues in tests.
# Note: The countdown=0 kwarg is set to ensure the method below does not attempt to access the course
# before the signal emitter has finished all operations. This is also necessary to ensure all tests pass.
tasks.update_xblocks_cache.apply_async([unicode(course_key)], countdown=0)
"""
Setup the signals on startup.
"""
from . import signals # pylint: disable=unused-import
"""
Tasks for bookmarks.
"""
import logging
from django.db import transaction
from celery.task import task # pylint: disable=import-error,no-name-in-module
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
from . import PathItem
log = logging.getLogger('edx.celery.task')
def _calculate_course_xblocks_data(course_key):
"""
Fetch data for all the blocks in the course.
This data consists of the display_name and path of the block.
"""
with modulestore().bulk_operations(course_key):
course = modulestore().get_course(course_key, depth=None)
blocks_info_dict = {}
# Collect display_name and children usage keys.
blocks_stack = [course]
while blocks_stack:
current_block = blocks_stack.pop()
children = current_block.get_children() if current_block.has_children else []
usage_id = unicode(current_block.scope_ids.usage_id)
block_info = {
'usage_key': current_block.scope_ids.usage_id,
'display_name': current_block.display_name,
'children_ids': [unicode(child.scope_ids.usage_id) for child in children]
}
blocks_info_dict[usage_id] = block_info
# Add this blocks children to the stack so that we can traverse them as well.
blocks_stack.extend(children)
# Set children
for block in blocks_info_dict.values():
block.setdefault('children', [])
for child_id in block['children_ids']:
block['children'].append(blocks_info_dict[child_id])
block.pop('children_ids', None)
# Calculate paths
def add_path_info(block_info, current_path):
"""Do a DFS and add paths info to each block_info."""
block_info.setdefault('paths', [])
block_info['paths'].append(current_path)
for child_block_info in block_info['children']:
add_path_info(child_block_info, current_path + [block_info])
add_path_info(blocks_info_dict[unicode(course.scope_ids.usage_id)], [])
return blocks_info_dict
def _paths_from_data(paths_data):
"""
Construct a list of paths from path data.
"""
paths = []
for path_data in paths_data:
paths.append([
PathItem(item['usage_key'], item['display_name']) for item in path_data
if item['usage_key'].block_type != 'course'
])
return [path for path in paths if path]
def paths_equal(paths_1, paths_2):
"""
Check if two paths are equivalent.
"""
if len(paths_1) != len(paths_2):
return False
for path_1, path_2 in zip(paths_1, paths_2):
if len(path_1) != len(path_2):
return False
for path_item_1, path_item_2 in zip(path_1, path_2):
if path_item_1.display_name != path_item_2.display_name:
return False
usage_key_1 = path_item_1.usage_key.replace(
course_key=modulestore().fill_in_run(path_item_1.usage_key.course_key)
)
usage_key_2 = path_item_1.usage_key.replace(
course_key=modulestore().fill_in_run(path_item_2.usage_key.course_key)
)
if usage_key_1 != usage_key_2:
return False
return True
def _update_xblocks_cache(course_key):
"""
Calculate the XBlock cache data for a course and update the XBlockCache table.
"""
from .models import XBlockCache
blocks_data = _calculate_course_xblocks_data(course_key)
def update_block_cache_if_needed(block_cache, block_data):
""" Compare block_cache object with data and update if there are differences. """
paths = _paths_from_data(block_data['paths'])
if block_cache.display_name != block_data['display_name'] or not paths_equal(block_cache.paths, paths):
log.info(u'Updating XBlockCache with usage_key: %s', unicode(block_cache.usage_key))
block_cache.display_name = block_data['display_name']
block_cache.paths = paths
block_cache.save()
with transaction.commit_on_success():
block_caches = XBlockCache.objects.filter(course_key=course_key)
for block_cache in block_caches:
block_data = blocks_data.pop(unicode(block_cache.usage_key), None)
if block_data:
update_block_cache_if_needed(block_cache, block_data)
for block_data in blocks_data.values():
with transaction.commit_on_success():
paths = _paths_from_data(block_data['paths'])
log.info(u'Creating XBlockCache with usage_key: %s', unicode(block_data['usage_key']))
block_cache, created = XBlockCache.objects.get_or_create(usage_key=block_data['usage_key'], defaults={
'course_key': course_key,
'display_name': block_data['display_name'],
'paths': paths,
})
if not created:
update_block_cache_if_needed(block_cache, block_data)
@task(name=u'openedx.core.djangoapps.bookmarks.tasks.update_xblock_cache')
def update_xblocks_cache(course_id):
"""
Update the XBlocks cache for a course.
Arguments:
course_id (String): The course_id of a course.
"""
# Ideally we'd like to accept a CourseLocator; however, CourseLocator is not JSON-serializable (by default) so
# Celery's delayed tasks fail to start. For this reason, callers should pass the course key as a Unicode string.
if not isinstance(course_id, basestring):
raise ValueError('course_id must be a string. {} is not acceptable.'.format(type(course_id)))
course_key = CourseKey.from_string(course_id)
log.info(u'Starting XBlockCaches update for course_key: %s', course_id)
_update_xblocks_cache(course_key)
log.info(u'Ending XBlockCaches update for course_key: %s', course_id)
...@@ -2,13 +2,13 @@ ...@@ -2,13 +2,13 @@
Factories for Bookmark models. Factories for Bookmark models.
""" """
import factory
from factory.django import DjangoModelFactory from factory.django import DjangoModelFactory
from factory import SubFactory
from functools import partial from functools import partial
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from ..models import Bookmark from ..models import Bookmark, XBlockCache
COURSE_KEY = SlashSeparatedCourseKey(u'edX', u'test_course', u'test') COURSE_KEY = SlashSeparatedCourseKey(u'edX', u'test_course', u'test')
LOCATION = partial(COURSE_KEY.make_usage_key, u'problem') LOCATION = partial(COURSE_KEY.make_usage_key, u'problem')
...@@ -18,8 +18,22 @@ class BookmarkFactory(DjangoModelFactory): ...@@ -18,8 +18,22 @@ class BookmarkFactory(DjangoModelFactory):
""" Simple factory class for generating Bookmark """ """ Simple factory class for generating Bookmark """
FACTORY_FOR = Bookmark FACTORY_FOR = Bookmark
user = SubFactory(UserFactory) user = factory.SubFactory(UserFactory)
course_key = COURSE_KEY course_key = COURSE_KEY
usage_key = LOCATION('usage_id') usage_key = LOCATION('usage_id')
display_name = ""
path = list() path = list()
xblock_cache = factory.SubFactory(
'openedx.core.djangoapps.bookmarks.tests.factories.XBlockCacheFactory',
course_key=factory.SelfAttribute('..course_key'),
usage_key=factory.SelfAttribute('..usage_key'),
)
class XBlockCacheFactory(DjangoModelFactory):
""" Simple factory class for generating XblockCache. """
FACTORY_FOR = XBlockCache
course_key = COURSE_KEY
usage_key = factory.Sequence(u'4x://edx/100/block/{0}'.format)
display_name = ''
paths = list()
"""
Tests for bookmarks api.
"""
import ddt
from mock import patch
from unittest import skipUnless
from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist
from opaque_keys.edx.keys import UsageKey
from xmodule.modulestore.exceptions import ItemNotFoundError
from .. import api
from ..models import Bookmark
from .test_models import BookmarksTestsBase
class BookmarkApiEventTestMixin(object):
""" Mixin for verifying that bookmark api events were emitted during a test. """
def assert_bookmark_event_emitted(self, mock_tracker, event_name, **kwargs):
""" Assert that an event has been emitted. """
mock_tracker.assert_any_call(
event_name,
kwargs,
)
def assert_no_events_were_emitted(self, mock_tracker):
"""
Assert no events were emitted.
"""
self.assertFalse(mock_tracker.called) # pylint: disable=maybe-no-member
@ddt.ddt
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
"""
These tests cover the parts of the API methods.
"""
def setUp(self):
super(BookmarksAPITests, self).setUp()
def test_get_bookmark(self):
"""
Verifies that get_bookmark returns data as expected.
"""
bookmark_data = api.get_bookmark(user=self.user, usage_key=self.sequential_1.location)
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmark_data)
# With Optional fields.
with self.assertNumQueries(1):
bookmark_data = api.get_bookmark(
user=self.user,
usage_key=self.sequential_1.location,
fields=self.ALL_FIELDS
)
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmark_data, check_optional_fields=True)
def test_get_bookmark_raises_error(self):
"""
Verifies that get_bookmark raises error as expected.
"""
with self.assertNumQueries(1):
with self.assertRaises(ObjectDoesNotExist):
api.get_bookmark(user=self.other_user, usage_key=self.vertical_1.location)
@ddt.data(
1, 10, 100
)
def test_get_bookmarks(self, count):
"""
Verifies that get_bookmarks returns data as expected.
"""
course, __, bookmarks = self.create_course_with_bookmarks_count(count)
# Without course key.
with self.assertNumQueries(1):
bookmarks_data = api.get_bookmarks(user=self.user)
self.assertEqual(len(bookmarks_data), count + 3)
# Assert them in ordered manner.
self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0])
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmarks_data[-1])
self.assert_bookmark_data_is_valid(self.bookmark_2, bookmarks_data[-2])
# Without course key, with optional fields.
with self.assertNumQueries(1):
bookmarks_data = api.get_bookmarks(user=self.user, fields=self.ALL_FIELDS)
self.assertEqual(len(bookmarks_data), count + 3)
self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0])
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmarks_data[-1])
# With course key.
with self.assertNumQueries(1):
bookmarks_data = api.get_bookmarks(user=self.user, course_key=course.id)
self.assertEqual(len(bookmarks_data), count)
self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0])
self.assert_bookmark_data_is_valid(bookmarks[0], bookmarks_data[-1])
# With course key, with optional fields.
with self.assertNumQueries(1):
bookmarks_data = api.get_bookmarks(user=self.user, course_key=course.id, fields=self.ALL_FIELDS)
self.assertEqual(len(bookmarks_data), count)
self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0])
self.assert_bookmark_data_is_valid(bookmarks[0], bookmarks_data[-1])
# Without Serialized.
with self.assertNumQueries(1):
bookmarks = api.get_bookmarks(user=self.user, course_key=course.id, serialized=False)
self.assertEqual(len(bookmarks), count)
self.assertTrue(bookmarks.model is Bookmark) # pylint: disable=no-member
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
def test_create_bookmark(self, mock_tracker):
"""
Verifies that create_bookmark create & returns data as expected.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
with self.assertNumQueries(4):
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_2.location)
self.assert_bookmark_event_emitted(
mock_tracker,
event_name='edx.bookmark.added',
course_id=unicode(self.course_id),
bookmark_id=bookmark_data['id'],
component_type=self.vertical_2.location.block_type,
component_usage_id=unicode(self.vertical_2.location),
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 3)
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
def test_create_bookmark_do_not_create_duplicates(self, mock_tracker):
"""
Verifies that create_bookmark do not create duplicate bookmarks.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 2)
with self.assertNumQueries(4):
bookmark_data = api.create_bookmark(user=self.user, usage_key=self.vertical_2.location)
self.assert_bookmark_event_emitted(
mock_tracker,
event_name='edx.bookmark.added',
course_id=unicode(self.course_id),
bookmark_id=bookmark_data['id'],
component_type=self.vertical_2.location.block_type,
component_usage_id=unicode(self.vertical_2.location),
)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 3)
mock_tracker.reset_mock()
with self.assertNumQueries(4):
bookmark_data_2 = api.create_bookmark(user=self.user, usage_key=self.vertical_2.location)
self.assertEqual(len(api.get_bookmarks(user=self.user, course_key=self.course.id)), 3)
self.assertEqual(bookmark_data, bookmark_data_2)
self.assert_no_events_were_emitted(mock_tracker)
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
def test_create_bookmark_raises_error(self, mock_tracker):
"""
Verifies that create_bookmark raises error as expected.
"""
with self.assertNumQueries(0):
with self.assertRaises(ItemNotFoundError):
api.create_bookmark(user=self.user, usage_key=UsageKey.from_string('i4x://brb/100/html/340ef1771a0940'))
self.assert_no_events_were_emitted(mock_tracker)
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
def test_delete_bookmark(self, mock_tracker):
"""
Verifies that delete_bookmark removes bookmark as expected.
"""
self.assertEqual(len(api.get_bookmarks(user=self.user)), 3)
with self.assertNumQueries(3):
api.delete_bookmark(user=self.user, usage_key=self.sequential_1.location)
self.assert_bookmark_event_emitted(
mock_tracker,
event_name='edx.bookmark.removed',
course_id=unicode(self.course_id),
bookmark_id=self.bookmark_1.resource_id,
component_type=self.sequential_1.location.block_type,
component_usage_id=unicode(self.sequential_1.location),
)
bookmarks_data = api.get_bookmarks(user=self.user)
self.assertEqual(len(bookmarks_data), 2)
self.assertNotEqual(unicode(self.sequential_1.location), bookmarks_data[0]['usage_id'])
self.assertNotEqual(unicode(self.sequential_1.location), bookmarks_data[1]['usage_id'])
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
def test_delete_bookmark_raises_error(self, mock_tracker):
"""
Verifies that delete_bookmark raises error as expected.
"""
with self.assertNumQueries(1):
with self.assertRaises(ObjectDoesNotExist):
api.delete_bookmark(user=self.other_user, usage_key=self.vertical_1.location)
self.assert_no_events_were_emitted(mock_tracker)
"""
Tests for Bookmarks models.
"""
from contextlib import contextmanager
import datetime
import ddt
from freezegun import freeze_time
import mock
import pytz
from unittest import skipUnless
from django.conf import settings
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.factories import check_mongo_calls, CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from student.tests.factories import AdminFactory, UserFactory
from .. import DEFAULT_FIELDS, OPTIONAL_FIELDS, PathItem
from ..models import Bookmark, XBlockCache, parse_path_data
from .factories import BookmarkFactory
EXAMPLE_USAGE_KEY_1 = u'i4x://org.15/course_15/chapter/Week_1'
EXAMPLE_USAGE_KEY_2 = u'i4x://org.15/course_15/chapter/Week_2'
noop_contextmanager = contextmanager(lambda x: (yield)) # pylint: disable=invalid-name
class BookmarksTestsBase(ModuleStoreTestCase):
"""
Test the Bookmark model.
"""
ALL_FIELDS = DEFAULT_FIELDS + OPTIONAL_FIELDS
STORE_TYPE = ModuleStoreEnum.Type.mongo
TEST_PASSWORD = 'test'
def setUp(self):
super(BookmarksTestsBase, self).setUp()
self.admin = AdminFactory()
self.user = UserFactory.create(password=self.TEST_PASSWORD)
self.other_user = UserFactory.create(password=self.TEST_PASSWORD)
self.setup_test_data(self.STORE_TYPE)
def setup_test_data(self, store_type=ModuleStoreEnum.Type.mongo):
""" Create courses and add some test blocks. """
with self.store.default_store(store_type):
self.course = CourseFactory.create(display_name='An Introduction to API Testing')
self.course_id = unicode(self.course.id)
if store_type == ModuleStoreEnum.Type.mongo:
bulk_operations_manager = self.store.bulk_operations
else:
bulk_operations_manager = noop_contextmanager
with bulk_operations_manager(self.course.id):
self.chapter_1 = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 1'
)
self.chapter_2 = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 2'
)
self.sequential_1 = ItemFactory.create(
parent_location=self.chapter_1.location, category='sequential', display_name='Lesson 1'
)
self.sequential_2 = ItemFactory.create(
parent_location=self.chapter_1.location, category='sequential', display_name='Lesson 2'
)
self.vertical_1 = ItemFactory.create(
parent_location=self.sequential_1.location, category='vertical', display_name='Subsection 1'
)
self.vertical_2 = ItemFactory.create(
parent_location=self.sequential_2.location, category='vertical', display_name='Subsection 2'
)
self.vertical_3 = ItemFactory.create(
parent_location=self.sequential_2.location, category='vertical', display_name='Subsection 3'
)
self.html_1 = ItemFactory.create(
parent_location=self.vertical_2.location, category='html', display_name='Details 1'
)
self.path = [
PathItem(self.chapter_1.location, self.chapter_1.display_name),
PathItem(self.sequential_2.location, self.sequential_2.display_name),
]
self.bookmark_1 = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=self.sequential_1.location,
xblock_cache=XBlockCache.create({
'display_name': self.sequential_1.display_name,
'usage_key': self.sequential_1.location,
}),
)
self.bookmark_2 = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=self.sequential_2.location,
xblock_cache=XBlockCache.create({
'display_name': self.sequential_2.display_name,
'usage_key': self.sequential_2.location,
}),
)
self.other_course = CourseFactory.create(display_name='An Introduction to API Testing 2')
with bulk_operations_manager(self.other_course.id):
self.other_chapter_1 = ItemFactory.create(
parent_location=self.other_course.location, category='chapter', display_name='Other Week 1'
)
self.other_sequential_1 = ItemFactory.create(
parent_location=self.other_chapter_1.location, category='sequential', display_name='Other Lesson 1'
)
self.other_sequential_2 = ItemFactory.create(
parent_location=self.other_chapter_1.location, category='sequential', display_name='Other Lesson 2'
)
self.other_vertical_1 = ItemFactory.create(
parent_location=self.other_sequential_1.location, category='vertical', display_name='Other Subsection 1'
)
self.other_vertical_2 = ItemFactory.create(
parent_location=self.other_sequential_1.location, category='vertical', display_name='Other Subsection 2'
)
# self.other_vertical_1 has two parents
self.other_sequential_2.children.append(self.other_vertical_1.location)
modulestore().update_item(self.other_sequential_2, self.admin.id) # pylint: disable=no-member
self.other_bookmark_1 = BookmarkFactory.create(
user=self.user,
course_key=unicode(self.other_course.id),
usage_key=self.other_vertical_1.location,
xblock_cache=XBlockCache.create({
'display_name': self.other_vertical_1.display_name,
'usage_key': self.other_vertical_1.location,
}),
)
def create_course_with_blocks(self, children_per_block=1, depth=1, store_type=ModuleStoreEnum.Type.mongo):
"""
Create a course and add blocks.
"""
with self.store.default_store(store_type):
course = CourseFactory.create()
display_name = 0
if store_type == ModuleStoreEnum.Type.mongo:
bulk_operations_manager = self.store.bulk_operations
else:
bulk_operations_manager = noop_contextmanager
with bulk_operations_manager(course.id):
blocks_at_next_level = [course]
for __ in range(depth):
blocks_at_current_level = blocks_at_next_level
blocks_at_next_level = []
for block in blocks_at_current_level:
for __ in range(children_per_block):
blocks_at_next_level += [ItemFactory.create(
parent_location=block.scope_ids.usage_id, display_name=unicode(display_name)
)]
display_name += 1
return course
def create_course_with_bookmarks_count(self, count, store_type=ModuleStoreEnum.Type.mongo):
"""
Create a course, add some content and add bookmarks.
"""
with self.store.default_store(store_type):
course = CourseFactory.create()
if store_type == ModuleStoreEnum.Type.mongo:
bulk_operations_manager = self.store.bulk_operations
else:
bulk_operations_manager = noop_contextmanager
with bulk_operations_manager(course.id):
blocks = [ItemFactory.create(
parent_location=course.location, category='chapter', display_name=unicode(index)
) for index in range(count)]
bookmarks = [BookmarkFactory.create(
user=self.user,
course_key=course.id,
usage_key=block.location,
xblock_cache=XBlockCache.create({
'display_name': block.display_name,
'usage_key': block.location,
}),
) for block in blocks]
return course, blocks, bookmarks
def assert_bookmark_model_is_valid(self, bookmark, bookmark_data):
"""
Assert that the attributes of the bookmark model were set correctly.
"""
self.assertEqual(bookmark.user, bookmark_data['user'])
self.assertEqual(bookmark.course_key, bookmark_data['course_key'])
self.assertEqual(unicode(bookmark.usage_key), unicode(bookmark_data['usage_key']))
self.assertEqual(bookmark.resource_id, u"{},{}".format(bookmark_data['user'], bookmark_data['usage_key']))
self.assertEqual(bookmark.display_name, bookmark_data['display_name'])
self.assertEqual(bookmark.path, self.path)
self.assertIsNotNone(bookmark.created)
self.assertEqual(bookmark.xblock_cache.course_key, bookmark_data['course_key'])
self.assertEqual(bookmark.xblock_cache.display_name, bookmark_data['display_name'])
def assert_bookmark_data_is_valid(self, bookmark, bookmark_data, check_optional_fields=False):
"""
Assert that the bookmark data matches the data in the model.
"""
self.assertEqual(bookmark_data['id'], bookmark.resource_id)
self.assertEqual(bookmark_data['course_id'], unicode(bookmark.course_key))
self.assertEqual(bookmark_data['usage_id'], unicode(bookmark.usage_key))
self.assertEqual(bookmark_data['block_type'], unicode(bookmark.usage_key.block_type))
self.assertIsNotNone(bookmark_data['created'])
if check_optional_fields:
self.assertEqual(bookmark_data['display_name'], bookmark.display_name)
self.assertEqual(bookmark_data['path'], bookmark.path)
@ddt.ddt
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
class BookmarkModelTests(BookmarksTestsBase):
"""
Test the Bookmark model.
"""
def get_bookmark_data(self, block, user=None):
"""
Returns bookmark data for testing.
"""
return {
'user': user or self.user,
'usage_key': block.location,
'course_key': block.location.course_key,
'display_name': block.display_name,
}
@ddt.data(
(ModuleStoreEnum.Type.mongo, 'course', [], 3),
(ModuleStoreEnum.Type.mongo, 'chapter_1', [], 3),
(ModuleStoreEnum.Type.mongo, 'sequential_1', ['chapter_1'], 4),
(ModuleStoreEnum.Type.mongo, 'vertical_1', ['chapter_1', 'sequential_1'], 5),
(ModuleStoreEnum.Type.mongo, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 6),
(ModuleStoreEnum.Type.split, 'course', [], 3),
(ModuleStoreEnum.Type.split, 'chapter_1', [], 2),
(ModuleStoreEnum.Type.split, 'sequential_1', ['chapter_1'], 2),
(ModuleStoreEnum.Type.split, 'vertical_1', ['chapter_1', 'sequential_1'], 2),
(ModuleStoreEnum.Type.split, 'html_1', ['chapter_1', 'sequential_2', 'vertical_2'], 2),
)
@ddt.unpack
def test_path_and_queries_on_create(self, store_type, block_to_bookmark, ancestors_attrs, expected_mongo_calls):
"""
In case of mongo, 1 query is used to fetch the block, and 2 by path_to_location(), and then
1 query per parent in path is needed to fetch the parent blocks.
"""
self.setup_test_data(store_type)
user = UserFactory.create()
expected_path = [PathItem(
usage_key=getattr(self, ancestor_attr).location, display_name=getattr(self, ancestor_attr).display_name
) for ancestor_attr in ancestors_attrs]
bookmark_data = self.get_bookmark_data(getattr(self, block_to_bookmark), user=user)
with check_mongo_calls(expected_mongo_calls):
bookmark, __ = Bookmark.create(bookmark_data)
self.assertEqual(bookmark.path, expected_path)
self.assertIsNotNone(bookmark.xblock_cache)
self.assertEqual(bookmark.xblock_cache.paths, [])
def test_create_bookmark_success(self):
"""
Tests creation of bookmark.
"""
bookmark_data = self.get_bookmark_data(self.vertical_2)
bookmark, __ = Bookmark.create(bookmark_data)
self.assert_bookmark_model_is_valid(bookmark, bookmark_data)
bookmark_data_different_values = self.get_bookmark_data(self.vertical_2)
bookmark_data_different_values['display_name'] = 'Introduction Video'
bookmark2, __ = Bookmark.create(bookmark_data_different_values)
# The bookmark object already created should have been returned without modifications.
self.assertEqual(bookmark, bookmark2)
self.assertEqual(bookmark.xblock_cache, bookmark2.xblock_cache)
self.assert_bookmark_model_is_valid(bookmark2, bookmark_data)
bookmark_data_different_user = self.get_bookmark_data(self.vertical_2)
bookmark_data_different_user['user'] = UserFactory.create()
bookmark3, __ = Bookmark.create(bookmark_data_different_user)
self.assertNotEqual(bookmark, bookmark3)
self.assert_bookmark_model_is_valid(bookmark3, bookmark_data_different_user)
@ddt.data(
(-30, [[PathItem(EXAMPLE_USAGE_KEY_1, '1')]], 1),
(30, None, 2),
(30, [], 2),
(30, [[PathItem(EXAMPLE_USAGE_KEY_1, '1')]], 1),
(30, [[PathItem(EXAMPLE_USAGE_KEY_1, '1')], [PathItem(EXAMPLE_USAGE_KEY_2, '2')]], 2),
)
@ddt.unpack
@mock.patch('openedx.core.djangoapps.bookmarks.models.Bookmark.get_path')
def test_path(self, seconds_delta, paths, get_path_call_count, mock_get_path):
block_path = [PathItem(UsageKey.from_string(EXAMPLE_USAGE_KEY_1), '1')]
mock_get_path.return_value = block_path
html = ItemFactory.create(
parent_location=self.other_chapter_1.location, category='html', display_name='Other Lesson 1'
)
bookmark_data = self.get_bookmark_data(html)
bookmark, __ = Bookmark.create(bookmark_data)
self.assertIsNotNone(bookmark.xblock_cache)
modification_datetime = datetime.datetime.now(pytz.utc) + datetime.timedelta(seconds=seconds_delta)
with freeze_time(modification_datetime):
bookmark.xblock_cache.paths = paths
bookmark.xblock_cache.save()
self.assertEqual(bookmark.path, block_path)
self.assertEqual(mock_get_path.call_count, get_path_call_count)
@ddt.data(
(ModuleStoreEnum.Type.mongo, 2, 2, 2),
(ModuleStoreEnum.Type.mongo, 4, 2, 2),
(ModuleStoreEnum.Type.mongo, 6, 2, 2),
(ModuleStoreEnum.Type.mongo, 2, 3, 3),
(ModuleStoreEnum.Type.mongo, 4, 3, 3),
(ModuleStoreEnum.Type.mongo, 6, 3, 3),
(ModuleStoreEnum.Type.mongo, 2, 4, 4),
(ModuleStoreEnum.Type.mongo, 4, 4, 4),
(ModuleStoreEnum.Type.split, 2, 2, 2),
(ModuleStoreEnum.Type.split, 4, 2, 2),
(ModuleStoreEnum.Type.split, 2, 3, 2),
(ModuleStoreEnum.Type.split, 4, 3, 2),
(ModuleStoreEnum.Type.split, 2, 4, 2),
)
@ddt.unpack
def test_get_path_queries(self, store_type, children_per_block, depth, expected_mongo_calls):
"""
In case of mongo, 2 queries are used by path_to_location(), and then
1 query per parent in path is needed to fetch the parent blocks.
"""
course = self.create_course_with_blocks(children_per_block, depth, store_type)
# Find a leaf block.
block = modulestore().get_course(course.id, depth=None)
for __ in range(depth - 1):
children = block.get_children()
block = children[-1]
with check_mongo_calls(expected_mongo_calls):
path = Bookmark.get_path(block.location)
self.assertEqual(len(path), depth - 2)
def test_get_path_in_case_of_exceptions(self):
user = UserFactory.create()
# Block does not exist
usage_key = UsageKey.from_string('i4x://edX/apis/html/interactive')
usage_key.replace(course_key=self.course.id)
self.assertEqual(Bookmark.get_path(usage_key), [])
# Block is an orphan
self.other_sequential_1.children = []
modulestore().update_item(self.other_sequential_1, self.admin.id) # pylint: disable=no-member
bookmark_data = self.get_bookmark_data(self.other_vertical_2, user=user)
bookmark, __ = Bookmark.create(bookmark_data)
self.assertEqual(bookmark.path, [])
self.assertIsNotNone(bookmark.xblock_cache)
self.assertEqual(bookmark.xblock_cache.paths, [])
# Parent block could not be retrieved
with mock.patch('openedx.core.djangoapps.bookmarks.models.search.path_to_location') as mock_path_to_location:
mock_path_to_location.return_value = [usage_key]
bookmark_data = self.get_bookmark_data(self.other_sequential_1, user=user)
bookmark, __ = Bookmark.create(bookmark_data)
self.assertEqual(bookmark.path, [])
@ddt.ddt
class XBlockCacheModelTest(ModuleStoreTestCase):
"""
Test the XBlockCache model.
"""
COURSE_KEY = CourseLocator(org='test', course='test', run='test')
CHAPTER1_USAGE_KEY = BlockUsageLocator(COURSE_KEY, block_type='chapter', block_id='chapter1')
SECTION1_USAGE_KEY = BlockUsageLocator(COURSE_KEY, block_type='section', block_id='section1')
SECTION2_USAGE_KEY = BlockUsageLocator(COURSE_KEY, block_type='section', block_id='section1')
VERTICAL1_USAGE_KEY = BlockUsageLocator(COURSE_KEY, block_type='vertical', block_id='sequential1')
PATH1 = [
[unicode(CHAPTER1_USAGE_KEY), 'Chapter 1'],
[unicode(SECTION1_USAGE_KEY), 'Section 1'],
]
PATH2 = [
[unicode(CHAPTER1_USAGE_KEY), 'Chapter 1'],
[unicode(SECTION2_USAGE_KEY), 'Section 2'],
]
def setUp(self):
super(XBlockCacheModelTest, self).setUp()
def assert_xblock_cache_data(self, xblock_cache, data):
"""
Assert that the XBlockCache object values match.
"""
self.assertEqual(xblock_cache.usage_key, data['usage_key'])
self.assertEqual(xblock_cache.course_key, data['usage_key'].course_key)
self.assertEqual(xblock_cache.display_name, data['display_name'])
self.assertEqual(xblock_cache._paths, data['_paths']) # pylint: disable=protected-access
self.assertEqual(xblock_cache.paths, [parse_path_data(path) for path in data['_paths']])
@ddt.data(
(
[
{'usage_key': VERTICAL1_USAGE_KEY, },
{'display_name': '', '_paths': [], },
],
[
{'usage_key': VERTICAL1_USAGE_KEY, 'display_name': 'Vertical 5', '_paths': [PATH2]},
{'_paths': []},
],
),
(
[
{'usage_key': VERTICAL1_USAGE_KEY, 'display_name': 'Vertical 4', '_paths': [PATH1]},
{},
],
[
{'usage_key': VERTICAL1_USAGE_KEY, 'display_name': 'Vertical 5', '_paths': [PATH2]},
{'_paths': [PATH1]},
],
),
)
def test_create(self, data):
"""
Test XBlockCache.create() constructs and updates objects correctly.
"""
for create_data, additional_data_to_expect in data:
xblock_cache = XBlockCache.create(create_data)
create_data.update(additional_data_to_expect)
self.assert_xblock_cache_data(xblock_cache, create_data)
@ddt.data(
([], [PATH1]),
([PATH1, PATH2], [PATH1]),
([PATH1], []),
)
@ddt.unpack
def test_paths(self, original_paths, updated_paths):
xblock_cache = XBlockCache.create({
'usage_key': self.VERTICAL1_USAGE_KEY,
'display_name': 'The end.',
'_paths': original_paths,
})
self.assertEqual(xblock_cache.paths, [parse_path_data(path) for path in original_paths])
xblock_cache.paths = [parse_path_data(path) for path in updated_paths]
xblock_cache.save()
xblock_cache = XBlockCache.objects.get(id=xblock_cache.id)
self.assertEqual(xblock_cache._paths, updated_paths) # pylint: disable=protected-access
self.assertEqual(xblock_cache.paths, [parse_path_data(path) for path in updated_paths])
"""
Tests for bookmark services.
"""
from unittest import skipUnless
from django.conf import settings
from opaque_keys.edx.keys import UsageKey
from ..services import BookmarksService
from .test_models import BookmarksTestsBase
@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
class BookmarksServiceTests(BookmarksTestsBase):
"""
Tests the Bookmarks service.
"""
def setUp(self):
super(BookmarksServiceTests, self).setUp()
self.bookmark_service = BookmarksService(user=self.user)
def test_get_bookmarks(self):
"""
Verifies get_bookmarks returns data as expected.
"""
with self.assertNumQueries(1):
bookmarks_data = self.bookmark_service.bookmarks(course_key=self.course.id)
self.assertEqual(len(bookmarks_data), 2)
self.assert_bookmark_data_is_valid(self.bookmark_2, bookmarks_data[0])
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmarks_data[1])
def test_is_bookmarked(self):
"""
Verifies is_bookmarked returns Bool as expected.
"""
with self.assertNumQueries(1):
self.assertTrue(self.bookmark_service.is_bookmarked(usage_key=self.sequential_1.location))
self.assertFalse(self.bookmark_service.is_bookmarked(usage_key=self.vertical_2.location))
self.assertTrue(self.bookmark_service.is_bookmarked(usage_key=self.sequential_2.location))
self.bookmark_service.set_bookmarked(usage_key=self.chapter_1.location)
with self.assertNumQueries(0):
self.assertTrue(self.bookmark_service.is_bookmarked(usage_key=self.chapter_1.location))
self.assertFalse(self.bookmark_service.is_bookmarked(usage_key=self.vertical_2.location))
# Removing a bookmark should result in the cache being updated on the next request
self.bookmark_service.unset_bookmarked(usage_key=self.chapter_1.location)
with self.assertNumQueries(0):
self.assertFalse(self.bookmark_service.is_bookmarked(usage_key=self.chapter_1.location))
self.assertFalse(self.bookmark_service.is_bookmarked(usage_key=self.vertical_2.location))
# Get bookmark that does not exist.
bookmark_service = BookmarksService(self.other_user)
with self.assertNumQueries(1):
self.assertFalse(bookmark_service.is_bookmarked(usage_key=self.sequential_1.location))
def test_set_bookmarked(self):
"""
Verifies set_bookmarked returns Bool as expected.
"""
# Assert False for item that does not exist.
with self.assertNumQueries(0):
self.assertFalse(
self.bookmark_service.set_bookmarked(usage_key=UsageKey.from_string("i4x://ed/ed/ed/interactive"))
)
with self.assertNumQueries(4):
self.assertTrue(self.bookmark_service.set_bookmarked(usage_key=self.vertical_2.location))
def test_unset_bookmarked(self):
"""
Verifies unset_bookmarked returns Bool as expected.
"""
with self.assertNumQueries(1):
self.assertFalse(
self.bookmark_service.unset_bookmarked(usage_key=UsageKey.from_string("i4x://ed/ed/ed/interactive"))
)
with self.assertNumQueries(3):
self.assertTrue(self.bookmark_service.unset_bookmarked(usage_key=self.sequential_1.location))
"""
Tests for tasks.
"""
import ddt
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.factories import check_mongo_calls
from ..models import XBlockCache
from ..tasks import _calculate_course_xblocks_data, _update_xblocks_cache
from .test_models import BookmarksTestsBase
@ddt.ddt
class XBlockCacheTaskTests(BookmarksTestsBase):
"""
Test the XBlockCache model.
"""
def setUp(self):
super(XBlockCacheTaskTests, self).setUp()
self.course_expected_cache_data = {
self.course.location: [
[],
], self.chapter_1.location: [
[
self.course.location,
],
], self.chapter_2.location: [
[
self.course.location,
],
], self.sequential_1.location: [
[
self.course.location,
self.chapter_1.location,
],
], self.sequential_2.location: [
[
self.course.location,
self.chapter_1.location,
],
], self.vertical_1.location: [
[
self.course.location,
self.chapter_1.location,
self.sequential_1.location,
],
], self.vertical_2.location: [
[
self.course.location,
self.chapter_1.location,
self.sequential_2.location,
],
], self.vertical_3.location: [
[
self.course.location,
self.chapter_1.location,
self.sequential_2.location,
],
],
}
self.other_course_expected_cache_data = { # pylint: disable=invalid-name
self.other_course.location: [
[],
], self.other_chapter_1.location: [
[
self.other_course.location,
],
], self.other_sequential_1.location: [
[
self.other_course.location,
self.other_chapter_1.location,
],
], self.other_sequential_2.location: [
[
self.other_course.location,
self.other_chapter_1.location,
],
], self.other_vertical_1.location: [
[
self.other_course.location,
self.other_chapter_1.location,
self.other_sequential_1.location,
],
[
self.other_course.location,
self.other_chapter_1.location,
self.other_sequential_2.location,
]
], self.other_vertical_2.location: [
[
self.other_course.location,
self.other_chapter_1.location,
self.other_sequential_1.location,
],
],
}
@ddt.data(
(ModuleStoreEnum.Type.mongo, 2, 2, 3),
(ModuleStoreEnum.Type.mongo, 4, 2, 3),
(ModuleStoreEnum.Type.mongo, 2, 3, 4),
(ModuleStoreEnum.Type.mongo, 4, 3, 4),
(ModuleStoreEnum.Type.mongo, 2, 4, 5),
(ModuleStoreEnum.Type.mongo, 4, 4, 6),
(ModuleStoreEnum.Type.split, 2, 2, 3),
(ModuleStoreEnum.Type.split, 4, 2, 3),
(ModuleStoreEnum.Type.split, 2, 3, 3),
(ModuleStoreEnum.Type.split, 2, 4, 3),
)
@ddt.unpack
def test_calculate_course_xblocks_data_queries(self, store_type, children_per_block, depth, expected_mongo_calls):
course = self.create_course_with_blocks(children_per_block, depth, store_type)
with check_mongo_calls(expected_mongo_calls):
blocks_data = _calculate_course_xblocks_data(course.id)
self.assertGreater(len(blocks_data), children_per_block ** depth)
@ddt.data(
('course',),
('other_course',)
)
@ddt.unpack
def test_calculate_course_xblocks_data(self, course_attr):
"""
Test that the xblocks data is calculated correctly.
"""
course = getattr(self, course_attr)
blocks_data = _calculate_course_xblocks_data(course.id)
expected_cache_data = getattr(self, course_attr + '_expected_cache_data')
for usage_key, __ in expected_cache_data.items():
for path_index, path in enumerate(blocks_data[unicode(usage_key)]['paths']):
for path_item_index, path_item in enumerate(path):
self.assertEqual(
path_item['usage_key'], expected_cache_data[usage_key][path_index][path_item_index]
)
@ddt.data(
('course', 19),
('other_course', 13)
)
@ddt.unpack
def test_update_xblocks_cache(self, course_attr, expected_sql_queries):
"""
Test that the xblocks data is persisted correctly.
"""
course = getattr(self, course_attr)
with self.assertNumQueries(expected_sql_queries):
_update_xblocks_cache(course.id)
expected_cache_data = getattr(self, course_attr + '_expected_cache_data')
for usage_key, __ in expected_cache_data.items():
xblock_cache = XBlockCache.objects.get(usage_key=usage_key)
for path_index, path in enumerate(xblock_cache.paths):
for path_item_index, path_item in enumerate(path):
self.assertEqual(
path_item.usage_key, expected_cache_data[usage_key][path_index][path_item_index + 1]
)
with self.assertNumQueries(1):
_update_xblocks_cache(course.id)
...@@ -4,34 +4,29 @@ Tests for bookmark views. ...@@ -4,34 +4,29 @@ Tests for bookmark views.
import ddt import ddt
import json import json
from unittest import skipUnless
import urllib import urllib
from django.conf import settings
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from mock import patch from mock import patch
from rest_framework.test import APIClient from rest_framework.test import APIClient
from student.tests.factories import UserFactory
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from .test_models import BookmarksTestsBase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from .test_api import BookmarkApiEventTestMixin
from .factories import BookmarkFactory
# pylint: disable=no-member # pylint: disable=no-member
class BookmarksViewsTestsBase(BookmarksTestsBase, BookmarkApiEventTestMixin):
class BookmarksViewTestsMixin(ModuleStoreTestCase):
""" """
Mixin for bookmarks views tests. Base class for bookmarks views tests.
""" """
test_password = 'test'
def setUp(self): def setUp(self):
super(BookmarksViewTestsMixin, self).setUp() super(BookmarksViewsTestsBase, self).setUp()
self.anonymous_client = APIClient() self.anonymous_client = APIClient()
self.user = UserFactory.create(password=self.test_password)
self.create_test_data()
self.client = self.login_client(user=self.user) self.client = self.login_client(user=self.user)
def login_client(self, user): def login_client(self, user):
...@@ -39,84 +34,9 @@ class BookmarksViewTestsMixin(ModuleStoreTestCase): ...@@ -39,84 +34,9 @@ class BookmarksViewTestsMixin(ModuleStoreTestCase):
Helper method for getting the client and user and logging in. Returns client. Helper method for getting the client and user and logging in. Returns client.
""" """
client = APIClient() client = APIClient()
client.login(username=user.username, password=self.test_password) client.login(username=user.username, password=self.TEST_PASSWORD)
return client return client
def create_test_data(self):
"""
Creates the bookmarks test data.
"""
with self.store.default_store(ModuleStoreEnum.Type.split):
self.course = CourseFactory.create()
self.course_id = unicode(self.course.id)
chapter_1 = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 1'
)
sequential_1 = ItemFactory.create(
parent_location=chapter_1.location, category='sequential', display_name='Lesson 1'
)
self.vertical_1 = ItemFactory.create(
parent_location=sequential_1.location, category='vertical', display_name='Subsection 1'
)
self.bookmark_1 = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=self.vertical_1.location,
display_name=self.vertical_1.display_name
)
chapter_2 = ItemFactory.create(
parent_location=self.course.location, category='chapter', display_name='Week 2'
)
sequential_2 = ItemFactory.create(
parent_location=chapter_2.location, category='sequential', display_name='Lesson 2'
)
vertical_2 = ItemFactory.create(
parent_location=sequential_2.location, category='vertical', display_name='Subsection 2'
)
self.vertical_3 = ItemFactory.create(
parent_location=sequential_2.location, category='vertical', display_name='Subsection 3'
)
self.bookmark_2 = BookmarkFactory.create(
user=self.user,
course_key=self.course_id,
usage_key=vertical_2.location,
display_name=vertical_2.display_name
)
# Other Course
self.other_course = CourseFactory.create(display_name='An Introduction to API Testing 2')
other_chapter = ItemFactory.create(
parent_location=self.other_course.location, category='chapter', display_name='Other Week 1'
)
other_sequential = ItemFactory.create(
parent_location=other_chapter.location, category='sequential', display_name='Other Lesson 1'
)
self.other_vertical = ItemFactory.create(
parent_location=other_sequential.location, category='vertical', display_name='Other Subsection 1'
)
self.other_bookmark = BookmarkFactory.create(
user=self.user,
course_key=unicode(self.other_course.id),
usage_key=self.other_vertical.location,
display_name=self.other_vertical.display_name
)
def assert_valid_bookmark_response(self, response_data, bookmark, optional_fields=False):
"""
Determines if the given response data (dict) matches the specified bookmark.
"""
self.assertEqual(response_data['id'], '%s,%s' % (self.user.username, unicode(bookmark.usage_key)))
self.assertEqual(response_data['course_id'], unicode(bookmark.course_key))
self.assertEqual(response_data['usage_id'], unicode(bookmark.usage_key))
self.assertEqual(response_data['block_type'], unicode(bookmark.usage_key.block_type))
self.assertIsNotNone(response_data['created'])
if optional_fields:
self.assertEqual(response_data['display_name'], bookmark.display_name)
self.assertEqual(response_data['path'], bookmark.path)
def send_get(self, client, url, query_parameters=None, expected_status=200): def send_get(self, client, url, query_parameters=None, expected_status=200):
""" """
Helper method for sending a GET to the server. Verifies the expected status and returns the response. Helper method for sending a GET to the server. Verifies the expected status and returns the response.
...@@ -144,109 +64,145 @@ class BookmarksViewTestsMixin(ModuleStoreTestCase): ...@@ -144,109 +64,145 @@ class BookmarksViewTestsMixin(ModuleStoreTestCase):
@ddt.ddt @ddt.ddt
class BookmarksListViewTests(BookmarksViewTestsMixin): @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
class BookmarksListViewTests(BookmarksViewsTestsBase):
""" """
This contains the tests for GET & POST methods of bookmark.views.BookmarksListView class This contains the tests for GET & POST methods of bookmark.views.BookmarksListView class
GET /api/bookmarks/v1/bookmarks/?course_id={course_id1} GET /api/bookmarks/v1/bookmarks/?course_id={course_id1}
POST /api/bookmarks/v1/bookmarks POST /api/bookmarks/v1/bookmarks
""" """
def assert_bookmark_listed_event_emitted(self, mock_tracker, **kwargs):
""" Assert that edx.course.bookmark.listed event is emitted with correct data. """
mock_tracker.assert_any_call(
'edx.bookmark.listed',
kwargs
)
@ddt.data( @ddt.data(
('course_id={}', False), (1, False),
('course_id={}&fields=path,display_name', True), (10, False),
(25, False),
(1, True),
(10, True),
(25, True),
) )
@ddt.unpack @ddt.unpack
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @patch('eventtracking.tracker.emit')
def test_get_bookmarks_successfully(self, query_params, check_optionals, mock_tracker): def test_get_bookmarks_successfully(self, bookmarks_count, check_all_fields, mock_tracker):
""" """
Test that requesting bookmarks for a course returns records successfully in Test that requesting bookmarks for a course returns records successfully in
expected order without optional fields. expected order without optional fields.
""" """
response = self.send_get(
client=self.client, course, __, bookmarks = self.create_course_with_bookmarks_count(
url=reverse('bookmarks'), bookmarks_count, store_type=ModuleStoreEnum.Type.mongo
query_parameters=query_params.format(urllib.quote(self.course_id))
) )
bookmarks = response.data['results'] query_parameters = 'course_id={}&page_size={}'.format(urllib.quote(unicode(course.id)), 100)
if check_all_fields:
query_parameters += '&fields=path,display_name'
with self.assertNumQueries(7): # 2 queries for bookmark table.
response = self.send_get(
client=self.client,
url=reverse('bookmarks'),
query_parameters=query_parameters,
)
bookmarks_data = response.data['results']
self.assertEqual(len(bookmarks), 2) self.assertEqual(len(bookmarks_data), len(bookmarks))
self.assertEqual(response.data['count'], 2) self.assertEqual(response.data['count'], len(bookmarks))
self.assertEqual(response.data['num_pages'], 1) self.assertEqual(response.data['num_pages'], 1)
# As bookmarks are sorted by -created so we will compare in that order. # As bookmarks are sorted by -created so we will compare in that order.
self.assert_valid_bookmark_response(bookmarks[0], self.bookmark_2, optional_fields=check_optionals) self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0], check_optional_fields=check_all_fields)
self.assert_valid_bookmark_response(bookmarks[1], self.bookmark_1, optional_fields=check_optionals) self.assert_bookmark_data_is_valid(bookmarks[0], bookmarks_data[-1], check_optional_fields=check_all_fields)
self.assert_bookmark_listed_event_emitted( self.assert_bookmark_event_emitted(
mock_tracker, mock_tracker,
course_id=self.course_id, event_name='edx.bookmark.listed',
course_id=unicode(course.id),
list_type='per_course', list_type='per_course',
bookmarks_count=2, bookmarks_count=bookmarks_count,
page_size=10, page_size=100,
page_number=1 page_number=1
) )
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @ddt.data(
def test_get_bookmarks_with_pagination(self, mock_tracker): 10, 25
)
@patch('eventtracking.tracker.emit')
def test_get_bookmarks_with_pagination(self, bookmarks_count, mock_tracker):
""" """
Test that requesting bookmarks for a course return results with pagination 200 code. Test that requesting bookmarks for a course return results with pagination 200 code.
""" """
query_parameters = 'course_id={}&page_size=1'.format(urllib.quote(self.course_id))
response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters)
bookmarks = response.data['results'] course, __, bookmarks = self.create_course_with_bookmarks_count(
bookmarks_count, store_type=ModuleStoreEnum.Type.mongo
)
page_size = 5
query_parameters = 'course_id={}&page_size={}'.format(urllib.quote(unicode(course.id)), page_size)
with self.assertNumQueries(7): # 2 queries for bookmark table.
response = self.send_get(
client=self.client,
url=reverse('bookmarks'),
query_parameters=query_parameters
)
bookmarks_data = response.data['results']
# Pagination assertions. # Pagination assertions.
self.assertEqual(response.data['count'], 2) self.assertEqual(response.data['count'], bookmarks_count)
self.assertIn('page=2&page_size=1', response.data['next']) self.assertIn('page=2&page_size={}'.format(page_size), response.data['next'])
self.assertEqual(response.data['num_pages'], 2) self.assertEqual(response.data['num_pages'], bookmarks_count / page_size)
self.assertEqual(len(bookmarks), 1) self.assertEqual(len(bookmarks_data), min(bookmarks_count, page_size))
self.assert_valid_bookmark_response(bookmarks[0], self.bookmark_2) self.assert_bookmark_data_is_valid(bookmarks[-1], bookmarks_data[0])
self.assert_bookmark_listed_event_emitted( self.assert_bookmark_event_emitted(
mock_tracker, mock_tracker,
course_id=self.course_id, event_name='edx.bookmark.listed',
course_id=unicode(course.id),
list_type='per_course', list_type='per_course',
bookmarks_count=2, bookmarks_count=bookmarks_count,
page_size=1, page_size=page_size,
page_number=1 page_number=1
) )
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @patch('eventtracking.tracker.emit')
def test_get_bookmarks_with_invalid_data(self, mock_tracker): def test_get_bookmarks_with_invalid_data(self, mock_tracker):
""" """
Test that requesting bookmarks with invalid data returns 0 records. Test that requesting bookmarks with invalid data returns 0 records.
""" """
# Invalid course id. # Invalid course id.
response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters='course_id=invalid') with self.assertNumQueries(5): # No queries for bookmark table.
bookmarks = response.data['results'] response = self.send_get(
self.assertEqual(len(bookmarks), 0) client=self.client,
url=reverse('bookmarks'),
query_parameters='course_id=invalid'
)
bookmarks_data = response.data['results']
self.assertEqual(len(bookmarks_data), 0)
self.assertFalse(mock_tracker.emit.called) # pylint: disable=maybe-no-member self.assertFalse(mock_tracker.emit.called) # pylint: disable=maybe-no-member
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @patch('eventtracking.tracker.emit')
def test_get_all_bookmarks_when_course_id_not_given(self, mock_tracker): def test_get_all_bookmarks_when_course_id_not_given(self, mock_tracker):
""" """
Test that requesting bookmarks returns all records for that user. Test that requesting bookmarks returns all records for that user.
""" """
# Without course id we would return all the bookmarks for that user. # Without course id we would return all the bookmarks for that user.
response = self.send_get(client=self.client, url=reverse('bookmarks'))
bookmarks = response.data['results'] with self.assertNumQueries(7): # 2 queries for bookmark table.
self.assertEqual(len(bookmarks), 3) response = self.send_get(
self.assert_valid_bookmark_response(bookmarks[0], self.other_bookmark) client=self.client,
self.assert_valid_bookmark_response(bookmarks[1], self.bookmark_2) url=reverse('bookmarks')
self.assert_valid_bookmark_response(bookmarks[2], self.bookmark_1) )
bookmarks_data = response.data['results']
self.assert_bookmark_listed_event_emitted( self.assertEqual(len(bookmarks_data), 3)
self.assert_bookmark_data_is_valid(self.other_bookmark_1, bookmarks_data[0])
self.assert_bookmark_data_is_valid(self.bookmark_2, bookmarks_data[1])
self.assert_bookmark_data_is_valid(self.bookmark_1, bookmarks_data[2])
self.assert_bookmark_event_emitted(
mock_tracker, mock_tracker,
event_name='edx.bookmark.listed',
list_type='all_courses', list_type='all_courses',
bookmarks_count=3, bookmarks_count=3,
page_size=10, page_size=10,
...@@ -258,28 +214,32 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -258,28 +214,32 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
Test that an anonymous client (not logged in) cannot call GET or POST. Test that an anonymous client (not logged in) cannot call GET or POST.
""" """
query_parameters = 'course_id={}'.format(self.course_id) query_parameters = 'course_id={}'.format(self.course_id)
self.send_get( with self.assertNumQueries(1): # No queries for bookmark table.
client=self.anonymous_client, self.send_get(
url=reverse('bookmarks'), client=self.anonymous_client,
query_parameters=query_parameters, url=reverse('bookmarks'),
expected_status=401 query_parameters=query_parameters,
) expected_status=401
self.send_post( )
client=self.anonymous_client,
url=reverse('bookmarks'), with self.assertNumQueries(1): # No queries for bookmark table.
data={'usage_id': 'test'}, self.send_post(
expected_status=401 client=self.anonymous_client,
) url=reverse('bookmarks'),
data={'usage_id': 'test'},
expected_status=401
)
def test_post_bookmark_successfully(self): def test_post_bookmark_successfully(self):
""" """
Test that posting a bookmark successfully returns newly created data with 201 code. Test that posting a bookmark successfully returns newly created data with 201 code.
""" """
response = self.send_post( with self.assertNumQueries(9):
client=self.client, response = self.send_post(
url=reverse('bookmarks'), client=self.client,
data={'usage_id': unicode(self.vertical_3.location)} url=reverse('bookmarks'),
) data={'usage_id': unicode(self.vertical_3.location)}
)
# Assert Newly created bookmark. # Assert Newly created bookmark.
self.assertEqual(response.data['id'], '%s,%s' % (self.user.username, unicode(self.vertical_3.location))) self.assertEqual(response.data['id'], '%s,%s' % (self.user.username, unicode(self.vertical_3.location)))
...@@ -298,31 +258,34 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -298,31 +258,34 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
3) With empty request.DATA 3) With empty request.DATA
""" """
# Send usage_id with invalid format. # Send usage_id with invalid format.
response = self.send_post( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, response = self.send_post(
url=reverse('bookmarks'), client=self.client,
data={'usage_id': 'invalid'}, url=reverse('bookmarks'),
expected_status=400 data={'usage_id': 'invalid'},
) expected_status=400
)
self.assertEqual(response.data['user_message'], u'Invalid usage_id: invalid.') self.assertEqual(response.data['user_message'], u'Invalid usage_id: invalid.')
# Send data without usage_id. # Send data without usage_id.
response = self.send_post( with self.assertNumQueries(4): # No queries for bookmark table.
client=self.client, response = self.send_post(
url=reverse('bookmarks'), client=self.client,
data={'course_id': 'invalid'}, url=reverse('bookmarks'),
expected_status=400 data={'course_id': 'invalid'},
) expected_status=400
)
self.assertEqual(response.data['user_message'], u'Parameter usage_id not provided.') self.assertEqual(response.data['user_message'], u'Parameter usage_id not provided.')
self.assertEqual(response.data['developer_message'], u'Parameter usage_id not provided.') self.assertEqual(response.data['developer_message'], u'Parameter usage_id not provided.')
# Send empty data dictionary. # Send empty data dictionary.
response = self.send_post( with self.assertNumQueries(4): # No queries for bookmark table.
client=self.client, response = self.send_post(
url=reverse('bookmarks'), client=self.client,
data={}, url=reverse('bookmarks'),
expected_status=400 data={},
) expected_status=400
)
self.assertEqual(response.data['user_message'], u'No data provided.') self.assertEqual(response.data['user_message'], u'No data provided.')
self.assertEqual(response.data['developer_message'], u'No data provided.') self.assertEqual(response.data['developer_message'], u'No data provided.')
...@@ -330,12 +293,13 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -330,12 +293,13 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
""" """
Test that posting a bookmark for a block that does not exist returns a 400. Test that posting a bookmark for a block that does not exist returns a 400.
""" """
response = self.send_post( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, response = self.send_post(
url=reverse('bookmarks'), client=self.client,
data={'usage_id': 'i4x://arbi/100/html/340ef1771a094090ad260ec940d04a21'}, url=reverse('bookmarks'),
expected_status=400 data={'usage_id': 'i4x://arbi/100/html/340ef1771a094090ad260ec940d04a21'},
) expected_status=400
)
self.assertEqual( self.assertEqual(
response.data['user_message'], response.data['user_message'],
u'Block with usage_id: i4x://arbi/100/html/340ef1771a094090ad260ec940d04a21 not found.' u'Block with usage_id: i4x://arbi/100/html/340ef1771a094090ad260ec940d04a21 not found.'
...@@ -349,11 +313,11 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -349,11 +313,11 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
""" """
Test that DELETE and PUT are not supported. Test that DELETE and PUT are not supported.
""" """
self.client.login(username=self.user.username, password=self.test_password) self.client.login(username=self.user.username, password=self.TEST_PASSWORD)
self.assertEqual(405, self.client.put(reverse('bookmarks')).status_code) self.assertEqual(405, self.client.put(reverse('bookmarks')).status_code)
self.assertEqual(405, self.client.delete(reverse('bookmarks')).status_code) self.assertEqual(405, self.client.delete(reverse('bookmarks')).status_code)
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @patch('eventtracking.tracker.emit')
@ddt.unpack @ddt.unpack
@ddt.data( @ddt.data(
{'page_size': -1, 'expected_bookmarks_count': 2, 'expected_page_size': 10, 'expected_page_number': 1}, {'page_size': -1, 'expected_bookmarks_count': 2, 'expected_page_size': 10, 'expected_page_number': 1},
...@@ -367,8 +331,9 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -367,8 +331,9 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters) self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters)
self.assert_bookmark_listed_event_emitted( self.assert_bookmark_event_emitted(
mock_tracker, mock_tracker,
event_name='edx.bookmark.listed',
course_id=self.course_id, course_id=self.course_id,
list_type='per_course', list_type='per_course',
bookmarks_count=expected_bookmarks_count, bookmarks_count=expected_bookmarks_count,
...@@ -376,13 +341,14 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -376,13 +341,14 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
page_number=expected_page_number page_number=expected_page_number
) )
@patch('lms.djangoapps.bookmarks.views.tracker.emit') @patch('openedx.core.djangoapps.bookmarks.views.eventtracking.tracker.emit')
def test_listed_event_for_page_number(self, mock_tracker): def test_listed_event_for_page_number(self, mock_tracker):
""" Test that edx.course.bookmark.listed event values are as expected when we request a specific page number """ """ Test that edx.course.bookmark.listed event values are as expected when we request a specific page number """
self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters='page_size=2&page=2') self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters='page_size=2&page=2')
self.assert_bookmark_listed_event_emitted( self.assert_bookmark_event_emitted(
mock_tracker, mock_tracker,
event_name='edx.bookmark.listed',
list_type='all_courses', list_type='all_courses',
bookmarks_count=3, bookmarks_count=3,
page_size=2, page_size=2,
...@@ -391,7 +357,8 @@ class BookmarksListViewTests(BookmarksViewTestsMixin): ...@@ -391,7 +357,8 @@ class BookmarksListViewTests(BookmarksViewTestsMixin):
@ddt.ddt @ddt.ddt
class BookmarksDetailViewTests(BookmarksViewTestsMixin): @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Tests only valid in LMS')
class BookmarksDetailViewTests(BookmarksViewsTestsBase):
""" """
This contains the tests for GET & DELETE methods of bookmark.views.BookmarksDetailView class This contains the tests for GET & DELETE methods of bookmark.views.BookmarksDetailView class
""" """
...@@ -400,47 +367,50 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -400,47 +367,50 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
('fields=path,display_name', True) ('fields=path,display_name', True)
) )
@ddt.unpack @ddt.unpack
def test_get_bookmark_successfully(self, query_params, check_optionals): def test_get_bookmark_successfully(self, query_params, check_optional_fields):
""" """
Test that requesting bookmark returns data with 200 code. Test that requesting bookmark returns data with 200 code.
""" """
response = self.send_get( with self.assertNumQueries(6): # 1 query for bookmark table.
client=self.client, response = self.send_get(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': self.user.username, 'usage_id': unicode(self.vertical_1.location)} 'bookmarks_detail',
), kwargs={'username': self.user.username, 'usage_id': unicode(self.sequential_1.location)}
query_parameters=query_params ),
) query_parameters=query_params
)
data = response.data data = response.data
self.assertIsNotNone(data) self.assertIsNotNone(data)
self.assert_valid_bookmark_response(data, self.bookmark_1, optional_fields=check_optionals) self.assert_bookmark_data_is_valid(self.bookmark_1, data, check_optional_fields=check_optional_fields)
def test_get_bookmark_that_belongs_to_other_user(self): def test_get_bookmark_that_belongs_to_other_user(self):
""" """
Test that requesting bookmark that belongs to other user returns 404 status code. Test that requesting bookmark that belongs to other user returns 404 status code.
""" """
self.send_get( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, self.send_get(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)} 'bookmarks_detail',
), kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)}
expected_status=404 ),
) expected_status=404
)
def test_get_bookmark_that_does_not_exist(self): def test_get_bookmark_that_does_not_exist(self):
""" """
Test that requesting bookmark that does not exist returns 404 status code. Test that requesting bookmark that does not exist returns 404 status code.
""" """
response = self.send_get( with self.assertNumQueries(6): # 1 query for bookmark table.
client=self.client, response = self.send_get(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': self.user.username, 'usage_id': 'i4x://arbi/100/html/340ef1771a0940'} 'bookmarks_detail',
), kwargs={'username': self.user.username, 'usage_id': 'i4x://arbi/100/html/340ef1771a0940'}
expected_status=404 ),
) expected_status=404
)
self.assertEqual( self.assertEqual(
response.data['user_message'], response.data['user_message'],
'Bookmark with usage_id: i4x://arbi/100/html/340ef1771a0940 does not exist.' 'Bookmark with usage_id: i4x://arbi/100/html/340ef1771a0940 does not exist.'
...@@ -454,14 +424,15 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -454,14 +424,15 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
""" """
Test that requesting bookmark with invalid usage id returns 400. Test that requesting bookmark with invalid usage id returns 400.
""" """
response = self.send_get( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, response = self.send_get(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': self.user.username, 'usage_id': 'i4x'} 'bookmarks_detail',
), kwargs={'username': self.user.username, 'usage_id': 'i4x'}
expected_status=404 ),
) expected_status=404
)
self.assertEqual(response.data['user_message'], u'Invalid usage_id: i4x.') self.assertEqual(response.data['user_message'], u'Invalid usage_id: i4x.')
def test_anonymous_access(self): def test_anonymous_access(self):
...@@ -469,16 +440,19 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -469,16 +440,19 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
Test that an anonymous client (not logged in) cannot call GET or DELETE. Test that an anonymous client (not logged in) cannot call GET or DELETE.
""" """
url = reverse('bookmarks_detail', kwargs={'username': self.user.username, 'usage_id': 'i4x'}) url = reverse('bookmarks_detail', kwargs={'username': self.user.username, 'usage_id': 'i4x'})
self.send_get( with self.assertNumQueries(4): # No queries for bookmark table.
client=self.anonymous_client, self.send_get(
url=url, client=self.anonymous_client,
expected_status=401 url=url,
) expected_status=401
self.send_delete( )
client=self.anonymous_client,
url=url, with self.assertNumQueries(1):
expected_status=401 self.send_delete(
) client=self.anonymous_client,
url=url,
expected_status=401
)
def test_delete_bookmark_successfully(self): def test_delete_bookmark_successfully(self):
""" """
...@@ -486,47 +460,49 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -486,47 +460,49 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
""" """
query_parameters = 'course_id={}'.format(urllib.quote(self.course_id)) query_parameters = 'course_id={}'.format(urllib.quote(self.course_id))
response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters) response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters)
data = response.data bookmarks_data = response.data['results']
bookmarks = data['results'] self.assertEqual(len(bookmarks_data), 2)
self.assertEqual(len(bookmarks), 2)
with self.assertNumQueries(7): # 2 queries for bookmark table.
self.send_delete( self.send_delete(
client=self.client, client=self.client,
url=reverse( url=reverse(
'bookmarks_detail', 'bookmarks_detail',
kwargs={'username': self.user.username, 'usage_id': unicode(self.vertical_1.location)} kwargs={'username': self.user.username, 'usage_id': unicode(self.sequential_1.location)}
)
) )
)
response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters) response = self.send_get(client=self.client, url=reverse('bookmarks'), query_parameters=query_parameters)
bookmarks = response.data['results'] bookmarks_data = response.data['results']
self.assertEqual(len(bookmarks), 1) self.assertEqual(len(bookmarks_data), 1)
def test_delete_bookmark_that_belongs_to_other_user(self): def test_delete_bookmark_that_belongs_to_other_user(self):
""" """
Test that delete bookmark that belongs to other user returns 404. Test that delete bookmark that belongs to other user returns 404.
""" """
self.send_delete( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, self.send_delete(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)} 'bookmarks_detail',
), kwargs={'username': 'other', 'usage_id': unicode(self.vertical_1.location)}
expected_status=404 ),
) expected_status=404
)
def test_delete_bookmark_that_does_not_exist(self): def test_delete_bookmark_that_does_not_exist(self):
""" """
Test that delete bookmark that does not exist returns 404. Test that delete bookmark that does not exist returns 404.
""" """
response = self.send_delete( with self.assertNumQueries(6): # 1 query for bookmark table.
client=self.client, response = self.send_delete(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': self.user.username, 'usage_id': 'i4x://arbi/100/html/340ef1771a0940'} 'bookmarks_detail',
), kwargs={'username': self.user.username, 'usage_id': 'i4x://arbi/100/html/340ef1771a0940'}
expected_status=404 ),
) expected_status=404
)
self.assertEqual( self.assertEqual(
response.data['user_message'], response.data['user_message'],
u'Bookmark with usage_id: i4x://arbi/100/html/340ef1771a0940 does not exist.' u'Bookmark with usage_id: i4x://arbi/100/html/340ef1771a0940 does not exist.'
...@@ -540,14 +516,15 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -540,14 +516,15 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
""" """
Test that delete bookmark with invalid usage id returns 400. Test that delete bookmark with invalid usage id returns 400.
""" """
response = self.send_delete( with self.assertNumQueries(5): # No queries for bookmark table.
client=self.client, response = self.send_delete(
url=reverse( client=self.client,
'bookmarks_detail', url=reverse(
kwargs={'username': self.user.username, 'usage_id': 'i4x'} 'bookmarks_detail',
), kwargs={'username': self.user.username, 'usage_id': 'i4x'}
expected_status=404 ),
) expected_status=404
)
self.assertEqual(response.data['user_message'], u'Invalid usage_id: i4x.') self.assertEqual(response.data['user_message'], u'Invalid usage_id: i4x.')
def test_unsupported_methods(self): def test_unsupported_methods(self):
...@@ -555,6 +532,9 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin): ...@@ -555,6 +532,9 @@ class BookmarksDetailViewTests(BookmarksViewTestsMixin):
Test that POST and PUT are not supported. Test that POST and PUT are not supported.
""" """
url = reverse('bookmarks_detail', kwargs={'username': self.user.username, 'usage_id': 'i4x'}) url = reverse('bookmarks_detail', kwargs={'username': self.user.username, 'usage_id': 'i4x'})
self.client.login(username=self.user.username, password=self.test_password) self.client.login(username=self.user.username, password=self.TEST_PASSWORD)
self.assertEqual(405, self.client.put(url).status_code) with self.assertNumQueries(5): # No queries for bookmark table.
self.assertEqual(405, self.client.post(url).status_code) self.assertEqual(405, self.client.put(url).status_code)
with self.assertNumQueries(4):
self.assertEqual(405, self.client.post(url).status_code)
...@@ -4,7 +4,7 @@ HTTP end-points for the Bookmarks API. ...@@ -4,7 +4,7 @@ HTTP end-points for the Bookmarks API.
For more information, see: For more information, see:
https://openedx.atlassian.net/wiki/display/TNL/Bookmarks+API https://openedx.atlassian.net/wiki/display/TNL/Bookmarks+API
""" """
from eventtracking import tracker import eventtracking
import logging import logging
from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ObjectDoesNotExist
...@@ -165,7 +165,10 @@ class BookmarksListView(ListCreateAPIView, BookmarksViewMixin): ...@@ -165,7 +165,10 @@ class BookmarksListView(ListCreateAPIView, BookmarksViewMixin):
else: else:
course_key = None course_key = None
return api.get_bookmarks(user=self.request.user, course_key=course_key, serialized=False) return api.get_bookmarks(
user=self.request.user, course_key=course_key,
fields=self.fields_to_return(self.request.QUERY_PARAMS), serialized=False
)
def paginate_queryset(self, queryset, page_size=None): def paginate_queryset(self, queryset, page_size=None):
""" Override GenericAPIView.paginate_queryset for the purpose of eventing """ """ Override GenericAPIView.paginate_queryset for the purpose of eventing """
...@@ -188,7 +191,7 @@ class BookmarksListView(ListCreateAPIView, BookmarksViewMixin): ...@@ -188,7 +191,7 @@ class BookmarksListView(ListCreateAPIView, BookmarksViewMixin):
event_data['list_type'] = 'per_course' event_data['list_type'] = 'per_course'
event_data['course_id'] = course_id event_data['course_id'] = course_id
tracker.emit('edx.bookmark.listed', event_data) eventtracking.tracker.emit('edx.bookmark.listed', event_data)
return page return page
......
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