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

test: Import linter: optionally enforce usage of a package's public API (#31903)

Browse Source 
* test: warn about dependencies from cms->openedx->lms and vice versa

* test: warn about importing from package's internal implementation code

* chore: Update some imports to use public APIs only

* chore: Update 'bookmarks' app to have stricter public API

* fix: we are sharing 'adapters' from olx_rest_api to content_staging
This commit is contained in:
Braden MacDonald
2023-04-20 11:34:51 -07:00
committed by GitHub
parent 72d8d830c9
commit 626f11f608
18 changed files with 490 additions and 263 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -69,7 +69,7 @@ from lms.djangoapps.grades.api import GradesUtilService
from lms.djangoapps.lms_xblock.field_data import LmsFieldData
from lms.djangoapps.lms_xblock.runtime import LmsModuleSystem, UserTagsService
from lms.djangoapps.verify_student.services import XBlockVerificationService
from openedx.core.djangoapps.bookmarks.services import BookmarksService
from openedx.core.djangoapps.bookmarks.api import BookmarksService
from openedx.core.djangoapps.crawlers.models import CrawlersConfig
from openedx.core.djangoapps.credit.services import CreditService
from openedx.core.djangoapps.util.user_utils import SystemUser

213
openedx/core/djangoapps/bookmarks/api.py
View File

@@ -1,204 +1,15 @@
"""
Bookmarks Python API.
Bookmarks Python public API.
"""
from django.conf import settings
from eventtracking import tracker
# pylint: disable=unused-import
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from . import DEFAULT_FIELDS, OPTIONAL_FIELDS
from .models import Bookmark
from .serializers import BookmarkSerializer
class BookmarksLimitReachedError(Exception):
"""
if try to create new bookmark when max limit of bookmarks already reached
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
def get_bookmark(user, usage_key, fields=None):
"""
Return data for a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
fields (list): List of field names the data should contain (optional).
Returns:
Dict.
Raises:
ObjectDoesNotExist: If a bookmark with the parameters does not exist.
"""
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
def get_bookmarks(user, course_key=None, fields=None, serialized=True):
"""
Return data for bookmarks of a user.
Arguments:
user (User): The user of the bookmarks.
course_key (CourseKey): The course_key of the bookmarks (optional).
fields (list): List of field names the data should contain (optional).
N/A if serialized is False.
serialized (bool): Whether to return a queryset or a serialized list of dicts.
Default is True.
Returns:
List of dicts if serialized is True else queryset.
"""
if user.is_authenticated:
bookmarks_queryset = Bookmark.objects.filter(user=user)
if 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('-id')
else:
bookmarks_queryset = Bookmark.objects.none()
if serialized:
return BookmarkSerializer(bookmarks_queryset, context={'fields': fields}, many=True).data
return bookmarks_queryset
def can_create_more(data):
"""
Determine if a new Bookmark can be created for the course
based on limit defined in django.conf.settings.MAX_BOOKMARKS_PER_COURSE
Arguments:
data (dict): The data to create the object with.
Returns:
Boolean
"""
data = dict(data)
user = data['user']
course_key = data['usage_key'].course_key
# User can create up to max_bookmarks_per_course bookmarks
if Bookmark.objects.filter(user=user, course_key=course_key).count() >= settings.MAX_BOOKMARKS_PER_COURSE:
return False
return True
def create_bookmark(user, usage_key):
"""
Create a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
Returns:
Dict.
Raises:
ItemNotFoundError: If no block exists for the usage_key.
BookmarksLimitReachedError: if try to create new bookmark when max limit of bookmarks already reached
"""
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
data = {
'user': user,
'usage_key': usage_key
}
if usage_key.course_key.run is None:
raise ItemNotFoundError
if not can_create_more(data):
raise BookmarksLimitReachedError
bookmark, created = Bookmark.create(data)
if created:
_track_event('edx.bookmark.added', bookmark)
return BookmarkSerializer(bookmark, context={'fields': DEFAULT_FIELDS + OPTIONAL_FIELDS}).data
def delete_bookmark(user, usage_key):
"""
Delete a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
Returns:
Dict.
Raises:
ObjectDoesNotExist: If a bookmark with the parameters does not exist.
"""
bookmark = Bookmark.objects.get(user=user, usage_key=usage_key)
bookmark.delete()
_track_event('edx.bookmark.removed', bookmark)
def delete_bookmarks(usage_key):
"""
Delete all bookmarks for usage_key.
Arguments:
usage_key (UsageKey): The usage_key of the bookmarks.
"""
units_keys = []
if usage_key.block_type == 'vertical':
units_keys.append(usage_key)
else:
# NOTE: Get all children for deleted block
descriptor = modulestore().get_item(usage_key)
for child in descriptor.get_children():
if usage_key.block_type == 'chapter':
units_keys += [unit.location for unit in child.get_children()]
else:
units_keys.append(child.location)
bookmarks = Bookmark.objects.filter(usage_key__in=units_keys)
# Emit removed bookmard event
for bookmark in bookmarks:
_track_event('edx.bookmark.removed', bookmark)
bookmarks.delete()
def _track_event(event_name, bookmark):
"""
Emit events for a bookmark.
Arguments:
event_name: name of event to track
bookmark: Bookmark object
"""
tracker.emit(
event_name,
{
'course_id': str(bookmark.course_key),
'bookmark_id': bookmark.resource_id,
'component_type': bookmark.usage_key.block_type,
'component_usage_id': str(bookmark.usage_key),
}
)
from .api_impl import (
BookmarksLimitReachedError,
get_bookmark,
get_bookmarks,
can_create_more,
create_bookmark,
delete_bookmark,
delete_bookmarks,
)
from .services import BookmarksService

215
openedx/core/djangoapps/bookmarks/api_impl.py Normal file
View File

@@ -0,0 +1,215 @@
"""
Bookmarks Python API.
"""
# Why does this file exist instead of just using api.py?
# (1) to expose BookmarkService (from .services import BookmarksService) in
# api.py; its implementation in services.py requires code from api.py. It
# caused a circular import error unless we put the API implementation into
# a separate file like this.
# (2) The code below imports a number of symbols that shouldn't be part of the
# public API (DEFAULT_FIELDS, OPTIONAL_FIELDS, Bookmark, BookmarkSerializer,
# modulestore, ItemNotFoundError, settings, tracker) and it's much easier
# to make them private this way (importing them into api_impl.py without
# changes) than it is to import them with an underscore prefix and change
# the code to use the prefixed imports.
from django.conf import settings
from eventtracking import tracker
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from . import DEFAULT_FIELDS, OPTIONAL_FIELDS
from .models import Bookmark
from .serializers import BookmarkSerializer
class BookmarksLimitReachedError(Exception):
"""
if try to create new bookmark when max limit of bookmarks already reached
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
def get_bookmark(user, usage_key, fields=None):
"""
Return data for a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
fields (list): List of field names the data should contain (optional).
Returns:
Dict.
Raises:
ObjectDoesNotExist: If a bookmark with the parameters does not exist.
"""
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
def get_bookmarks(user, course_key=None, fields=None, serialized=True):
"""
Return data for bookmarks of a user.
Arguments:
user (User): The user of the bookmarks.
course_key (CourseKey): The course_key of the bookmarks (optional).
fields (list): List of field names the data should contain (optional).
N/A if serialized is False.
serialized (bool): Whether to return a queryset or a serialized list of dicts.
Default is True.
Returns:
List of dicts if serialized is True else queryset.
"""
if user.is_authenticated:
bookmarks_queryset = Bookmark.objects.filter(user=user)
if 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('-id')
else:
bookmarks_queryset = Bookmark.objects.none()
if serialized:
return BookmarkSerializer(bookmarks_queryset, context={'fields': fields}, many=True).data
return bookmarks_queryset
def can_create_more(data):
"""
Determine if a new Bookmark can be created for the course
based on limit defined in django.conf.settings.MAX_BOOKMARKS_PER_COURSE
Arguments:
data (dict): The data to create the object with.
Returns:
Boolean
"""
data = dict(data)
user = data['user']
course_key = data['usage_key'].course_key
# User can create up to max_bookmarks_per_course bookmarks
if Bookmark.objects.filter(user=user, course_key=course_key).count() >= settings.MAX_BOOKMARKS_PER_COURSE:
return False
return True
def create_bookmark(user, usage_key):
"""
Create a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
Returns:
Dict.
Raises:
ItemNotFoundError: If no block exists for the usage_key.
BookmarksLimitReachedError: if try to create new bookmark when max limit of bookmarks already reached
"""
usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key))
data = {
'user': user,
'usage_key': usage_key
}
if usage_key.course_key.run is None:
raise ItemNotFoundError
if not can_create_more(data):
raise BookmarksLimitReachedError
bookmark, created = Bookmark.create(data)
if created:
_track_event('edx.bookmark.added', bookmark)
return BookmarkSerializer(bookmark, context={'fields': DEFAULT_FIELDS + OPTIONAL_FIELDS}).data
def delete_bookmark(user, usage_key):
"""
Delete a bookmark.
Arguments:
user (User): The user of the bookmark.
usage_key (UsageKey): The usage_key of the bookmark.
Returns:
Dict.
Raises:
ObjectDoesNotExist: If a bookmark with the parameters does not exist.
"""
bookmark = Bookmark.objects.get(user=user, usage_key=usage_key)
bookmark.delete()
_track_event('edx.bookmark.removed', bookmark)
def delete_bookmarks(usage_key):
"""
Delete all bookmarks for usage_key.
Arguments:
usage_key (UsageKey): The usage_key of the bookmarks.
"""
units_keys = []
if usage_key.block_type == 'vertical':
units_keys.append(usage_key)
else:
# NOTE: Get all children for deleted block
descriptor = modulestore().get_item(usage_key)
for child in descriptor.get_children():
if usage_key.block_type == 'chapter':
units_keys += [unit.location for unit in child.get_children()]
else:
units_keys.append(child.location)
bookmarks = Bookmark.objects.filter(usage_key__in=units_keys)
# Emit removed bookmard event
for bookmark in bookmarks:
_track_event('edx.bookmark.removed', bookmark)
bookmarks.delete()
def _track_event(event_name, bookmark):
"""
Emit events for a bookmark.
Arguments:
event_name: name of event to track
bookmark: Bookmark object
"""
tracker.emit(
event_name,
{
'course_id': str(bookmark.course_key),
'bookmark_id': bookmark.resource_id,
'component_type': bookmark.usage_key.block_type,
'component_usage_id': str(bookmark.usage_key),
}
)

2
openedx/core/djangoapps/bookmarks/services.py
View File

@@ -11,7 +11,7 @@ from edx_django_utils.cache import DEFAULT_REQUEST_CACHE
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from . import DEFAULT_FIELDS, api
from . import DEFAULT_FIELDS, api_impl as api
log = logging.getLogger(__name__)

18
openedx/core/djangoapps/bookmarks/tests/test_api.py
View File

@@ -111,7 +111,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
assert len(bookmarks) == count
assert bookmarks.model is Bookmark
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_create_bookmark(self, mock_tracker):
"""
Verifies that create_bookmark create & returns data as expected.
@@ -132,7 +132,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
assert len(api.get_bookmarks(user=self.user, course_key=self.course.id)) == 5
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_create_bookmark_do_not_create_duplicates(self, mock_tracker):
"""
Verifies that create_bookmark do not create duplicate bookmarks.
@@ -163,7 +163,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
self.assert_no_events_were_emitted(mock_tracker)
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_create_bookmark_raises_error(self, mock_tracker):
"""
Verifies that create_bookmark raises error as expected.
@@ -174,7 +174,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
self.assert_no_events_were_emitted(mock_tracker)
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
@patch('django.conf.settings.MAX_BOOKMARKS_PER_COURSE', 5)
def bookmark_more_than_limit_raise_error(self, mock_tracker):
"""
@@ -200,7 +200,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
api.create_bookmark(user=self.other_user, usage_key=blocks[-1].location)
assert len(api.get_bookmarks(user=self.other_user, course_key=blocks[(- 1)].location.course_key)) == 1
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_delete_bookmark(self, mock_tracker):
"""
Verifies that delete_bookmark removes bookmark as expected.
@@ -224,7 +224,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
assert str(self.sequential_1.location) != bookmarks_data[0]['usage_id']
assert str(self.sequential_1.location) != bookmarks_data[1]['usage_id']
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_delete_bookmarks_with_vertical_block_type(self, mock_tracker):
assert len(api.get_bookmarks(user=self.user)) == 5
@@ -241,8 +241,8 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
assert len(api.get_bookmarks(self.user)) == 4
@patch('openedx.core.djangoapps.bookmarks.api.modulestore')
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.modulestore')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_delete_bookmarks_with_chapter_block_type(self, mock_tracker, mocked_modulestore):
mocked_modulestore().get_item().get_children = Mock(
return_value=[Mock(get_children=Mock(return_value=[Mock(
@@ -263,7 +263,7 @@ class BookmarksAPITests(BookmarkApiEventTestMixin, BookmarksTestsBase):
)
assert len(api.get_bookmarks(self.user)) == 4
@patch('openedx.core.djangoapps.bookmarks.api.tracker.emit')
@patch('openedx.core.djangoapps.bookmarks.api_impl.tracker.emit')
def test_delete_bookmark_raises_error(self, mock_tracker):
"""
Verifies that delete_bookmark raises error as expected.

13
openedx/core/djangoapps/content_libraries/api.py
View File

@@ -92,10 +92,13 @@ from openedx.core.djangoapps.content_libraries.signals import (
LIBRARY_BLOCK_UPDATED,
LIBRARY_BLOCK_DELETED,
)
from openedx.core.djangoapps.olx_rest_api.block_serializer import XBlockSerializer
from openedx.core.djangoapps.xblock.api import get_block_display_name, load_block
from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl
from openedx.core.djangoapps.xblock.runtime.olx_parsing import XBlockInclude
from openedx.core.djangoapps.olx_rest_api.api import serialize_modulestore_block_for_blockstore
from openedx.core.djangoapps.xblock.api import (
get_block_display_name,
get_learning_context_impl,
load_block,
XBlockInclude,
)
from openedx.core.lib.blockstore_api import (
get_bundle,
get_bundles,
@@ -1258,7 +1261,7 @@ class EdxModulestoreImportClient(BaseEdxImportClient):
Get block OLX by serializing it from modulestore directly.
"""
block = self.modulestore.get_item(block_key)
data = XBlockSerializer(block)
data = serialize_modulestore_block_for_blockstore(block)
return {'olx': data.olx_str,
'static_files': {s.name: s for s in data.static_files}}

4
openedx/core/djangoapps/content_libraries/library_bundle.py
View File

@@ -10,11 +10,11 @@ from xblock.core import XBlock
from xblock.plugin import PluginMissingError
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
from openedx.core.djangoapps.xblock.runtime.blockstore_runtime import xml_for_definition
from openedx.core.djangoapps.xblock.runtime.olx_parsing import (
from openedx.core.djangoapps.xblock.api import (
BundleFormatException,
definition_for_include,
parse_xblock_include,
xml_for_definition,
)
from openedx.core.djangolib.blockstore_cache import (
BundleCache,

2
openedx/core/djangoapps/content_libraries/library_context.py
View File

@@ -13,7 +13,7 @@ from openedx.core.djangoapps.content_libraries.library_bundle import (
usage_for_child_include,
)
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
from openedx.core.djangoapps.xblock.learning_context import LearningContext
from openedx.core.djangoapps.xblock.api import LearningContext
log = logging.getLogger(__name__)

44
openedx/core/djangoapps/content_libraries/tests/base.py
View File

@@ -4,12 +4,10 @@ Tests for Blockstore-based Content Libraries
from contextlib import contextmanager
from io import BytesIO
from urllib.parse import urlencode
from unittest import mock, skipUnless
from urllib.parse import urlparse
from unittest import mock
from django.conf import settings
from django.test import LiveServerTestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
from organizations.models import Organization
from rest_framework.test import APITestCase, APIClient
@@ -20,6 +18,11 @@ from openedx.core.djangoapps.content_libraries.libraries_index import MAX_SIZE
from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED
from openedx.core.djangolib.testing.utils import skip_unless_cms
from openedx.core.lib import blockstore_api
from openedx.core.lib.blockstore_api.tests.base import (
BlockstoreAppTestMixin,
requires_blockstore,
requires_blockstore_app,
)
# Define the URLs here - don't use reverse() because we want to detect
# backwards-incompatible changes like changed URLs.
@@ -49,41 +52,6 @@ URL_BLOCK_METADATA_URL = '/api/xblock/v2/xblocks/{block_key}/'
URL_BLOCK_XBLOCK_HANDLER = '/api/xblock/v2/xblocks/{block_key}/handler/{user_id}-{secure_token}/{handler_name}/'
# Decorators for tests that require the blockstore service/app
requires_blockstore = skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
requires_blockstore_app = skipUnless(settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API, "Requires blockstore app")
class BlockstoreAppTestMixin:
"""
Sets up the environment for tests to be run using the installed Blockstore app.
"""
def setUp(self):
"""
Ensure there's an active request, so that bundle file URLs can be made absolute.
"""
super().setUp()
# Patch the blockstore get_current_request to use our live_server_url
mock.patch('blockstore.apps.api.methods.get_current_request',
mock.Mock(return_value=self._get_current_request())).start()
self.addCleanup(mock.patch.stopall)
def _get_current_request(self):
"""
Returns a request object using the live_server_url, if available.
"""
request_args = {}
if hasattr(self, 'live_server_url'):
live_server_url = urlparse(self.live_server_url)
name, port = live_server_url.netloc.split(':')
request_args['SERVER_NAME'] = name
request_args['SERVER_PORT'] = port or '80'
request_args['wsgi.url_scheme'] = live_server_url.scheme
return RequestFactory().request(**request_args)
def elasticsearch_test(func):
"""
Decorator for tests which connect to elasticsearch when needed

2
openedx/core/djangoapps/content_staging/block_serializer.py
View File

@@ -8,7 +8,7 @@ from collections import namedtuple
from lxml import etree
from openedx.core.djangoapps.olx_rest_api import adapters
from openedx.core.djangoapps.olx_rest_api.api import adapters
log = logging.getLogger(__name__)

22
openedx/core/djangoapps/olx_rest_api/api.py Normal file
View File

@@ -0,0 +1,22 @@
"""
Public Python API for the OLX REST API app
"""
from .block_serializer import XBlockSerializer as _XBlockSerializer
# pylint: disable=unused-import
# 'adapters' are _temporarily_ part of the public API to keep the code DRY until
# we can consolidate the two different block_serializer implementations in
# content_staging and olx_rest_api.
from . import adapters
def serialize_modulestore_block_for_blockstore(block):
"""
Given a modulestore XBlock (e.g. loaded using
modulestore.get_item(block_key)
), produce:
(1) A new definition ID for use in Blockstore
(2) an XML string defining the XBlock and referencing the IDs of its
children (but not containing the actual XML of its children)
(3) a list of any static files required by the XBlock and their URL
"""
return _XBlockSerializer(block)

16
openedx/core/djangoapps/xblock/api.py
View File

@@ -7,6 +7,7 @@ the older runtime.
Note that these views are only for interacting with existing blocks. Other
Studio APIs cover use cases like adding/deleting/editing blocks.
"""
# pylint: disable=unused-import
import logging
import threading
@@ -22,10 +23,21 @@ from xblock.exceptions import NoSuchViewError
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
from openedx.core.djangoapps.xblock.learning_context.manager import get_learning_context_impl
from openedx.core.djangoapps.xblock.runtime.blockstore_runtime import BlockstoreXBlockRuntime, xml_for_definition
from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntimeSystem
from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntimeSystem as _XBlockRuntimeSystem
from openedx.core.djangolib.blockstore_cache import BundleCache
from .utils import get_secure_token_for_xblock_handler, get_xblock_id_for_anonymous_user
# Made available as part of this package's public API:
from openedx.core.djangoapps.xblock.learning_context import LearningContext
from openedx.core.djangoapps.xblock.runtime.olx_parsing import (
BundleFormatException,
definition_for_include,
parse_xblock_include,
XBlockInclude,
)
# Implementation:
log = logging.getLogger(__name__)
@@ -54,7 +66,7 @@ def get_runtime_system():
runtime_class=BlockstoreXBlockRuntime,
)
params.update(get_xblock_app_config().get_runtime_system_params())
setattr(get_runtime_system, cache_name, XBlockRuntimeSystem(**params))
setattr(get_runtime_system, cache_name, _XBlockRuntimeSystem(**params))
return getattr(get_runtime_system, cache_name)

2
openedx/core/djangolib/tests/test_blockstore_cache.py
View File

@@ -5,7 +5,7 @@ from unittest.mock import patch
from django.test import TestCase
from openedx.core.djangolib.blockstore_cache import BundleCache
from openedx.core.djangoapps.content_libraries.tests.base import (
from openedx.core.lib.blockstore_api.tests.base import (
BlockstoreAppTestMixin,
requires_blockstore,
requires_blockstore_app,

42
openedx/core/lib/blockstore_api/tests/base.py Normal file
View File

@@ -0,0 +1,42 @@
"""
Common code for tests that work with Blockstore
"""
from unittest import mock, skipUnless
from urllib.parse import urlparse
from django.conf import settings
from django.test.client import RequestFactory
# Decorators for tests that require the blockstore service/app
requires_blockstore = skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
requires_blockstore_app = skipUnless(settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API, "Requires blockstore app")
class BlockstoreAppTestMixin:
"""
Sets up the environment for tests to be run using the installed Blockstore app.
"""
def setUp(self):
"""
Ensure there's an active request, so that bundle file URLs can be made absolute.
"""
super().setUp()
# Patch the blockstore get_current_request to use our live_server_url
mock.patch('blockstore.apps.api.methods.get_current_request',
mock.Mock(return_value=self._get_current_request())).start()
self.addCleanup(mock.patch.stopall)
def _get_current_request(self):
"""
Returns a request object using the live_server_url, if available.
"""
request_args = {}
if hasattr(self, 'live_server_url'):
live_server_url = urlparse(self.live_server_url)
name, port = live_server_url.netloc.split(':')
request_args['SERVER_NAME'] = name
request_args['SERVER_PORT'] = port or '80'
request_args['wsgi.url_scheme'] = live_server_url.scheme
return RequestFactory().request(**request_args)

2
openedx/core/lib/blockstore_api/tests/test_blockstore_api.py
View File

@@ -8,7 +8,7 @@ import pytest
from django.test import TestCase
from openedx.core.lib import blockstore_api as api
from openedx.core.djangoapps.content_libraries.tests.base import (
from openedx.core.lib.blockstore_api.tests.base import (
BlockstoreAppTestMixin,
requires_blockstore,
requires_blockstore_app,

0
openedx/testing/importlinter/__init__.py Normal file
View File

59
openedx/testing/importlinter/isolated_apps_contract.py Normal file
View File

@@ -0,0 +1,59 @@
"""
An importlinter contract that can flag imports of private APIs
"""
from importlinter import Contract, ContractCheck, fields, output
class IsolatedAppsContract(Contract):
"""
Contract that defines most of an 'app' (python package) as private, and
ensures that python code outside of the package doesn't import anything
other than the public API defined in the package's `api.py` file.
"""
isolated_apps = fields.ListField(subfield=fields.StringField())
# List of allowed modules (like ["api", "urls"] to allow "import x.api")
allowed_modules = fields.ListField(subfield=fields.StringField())
def check(self, graph, verbose):
forbidden_imports_found = []
for package in self.isolated_apps:
output.verbose_print(
verbose,
f"Getting import details for anything that imports {package}..."
)
modules = graph.find_descendants(package)
for module in modules:
# We have a list of modules like "api.py" that *are* allowed to be imported from anywhere:
for allowed_module in self.allowed_modules:
if module.endswith(f".{allowed_module}"):
break
else:
# See who is importing this:
importers = graph.find_modules_that_directly_import(module)
for importer in importers:
if importer.startswith(package):
continue # Ignore imports from within the same package
# Add this import to our list of contract violations:
import_details = graph.get_import_details(importer=importer, imported=module)
for import_detail in import_details:
forbidden_imports_found.append({**import_detail, "package": package})
return ContractCheck(
kept=not bool(forbidden_imports_found),
metadata={
'forbidden_imports_found': forbidden_imports_found,
}
)
def render_broken_contract(self, check):
for details in check.metadata['forbidden_imports_found']:
package = details['package']
importer = details['importer']
line_number = details['line_number']
line_contents = details['line_contents']
output.print_error(f'{importer}:{line_number}: imported from non-public API of {package}:')
output.indent_cursor()
output.print_error(line_contents)
output.new_line()

95
setup.cfg
View File

@@ -52,7 +52,12 @@ skip=
root_packages =
lms
cms
openedx
include_external_packages = True
contract_types =
# Our custom contract which checks that we're only importing from 'api.py'
# for participating packages.
isolated_apps: openedx.testing.importlinter.isolated_apps_contract.IsolatedAppsContract
[importlinter:contract:1]
name = lms and cms are independent
@@ -61,10 +66,16 @@ modules =
lms
cms
ignore_imports =
############################################################################
# lms side imports that we are ignoring for now
lms.djangoapps.course_home_api.outline.tests.test_view -> cms.djangoapps.contentstore.outlines
lms.djangoapps.courseware.plugins -> cms.djangoapps.contentstore.utils
lms.djangoapps.course_home_api.tests.utils -> cms.djangoapps.contentstore.outlines
# lms.djangoapps.instructor.tests.test_api & lms.djangoapps.instructor.tests.test_tools
# -> openedx.core.djangoapps.course_date_signals.handlers
# -> cms.djangoapps.contentstore.config.waffle
openedx.core.djangoapps.course_date_signals.handlers -> cms.djangoapps.contentstore.config.waffle
############################################################################
# cms side imports that we are ignoring for now
cms.djangoapps.contentstore.views.tests.test_block -> lms.djangoapps.lms_xblock.mixin
cms.envs.common -> lms.envs.common
@@ -73,3 +84,87 @@ ignore_imports =
cms.djangoapps.contentstore.views.preview -> lms.djangoapps.lms_xblock.field_data
cms.envs.common -> lms.djangoapps.lms_xblock.mixin
cms.envs.test -> lms.envs.test
# cms.djangoapps.contentstore.views.tests.test_group_configurations
# -> openedx.features.content_type_gating.helpers
# -> lms.djangoapps.courseware.masquerade
openedx.features.content_type_gating.helpers -> lms.djangoapps.courseware.masquerade
# cms.djangoapps.contentstore.utils
# -> openedx.core.djangoapps.django_comment_common.models
# -> openedx.core.djangoapps.course_groups.cohorts
# -> lms.djangoapps.courseware.courses
openedx.core.djangoapps.course_groups.cohorts -> lms.djangoapps.courseware.courses
# cms.djangoapps.models.settings.course_metadata
# -> openedx.features.course_experience
# -> openedx.features.course_experience.url_helpers
# -> lms.djangoapps.courseware.toggles
openedx.features.course_experience.url_helpers -> lms.djangoapps.courseware.toggles
# cms.djangoapps.contentstore.[various]
# -> openedx.features.content_type_gating.partitions
# -> lms.djangoapps.commerce.utils
openedx.features.content_type_gating.partitions -> lms.djangoapps.commerce.utils
# cms.djangoapps.contentstore.course_group_config
# -> openedx.core.djangoapps.course_groups.partition_scheme
# -> lms.djangoapps.courseware.masquerade
openedx.core.djangoapps.course_groups.partition_scheme -> lms.djangoapps.courseware.masquerade
# cms.djangoapps.contentstore.[various] & cms.djangoapps.coursegraph.[various]
# -> openedx.core.djangoapps.content.course_overviews.models
# -> lms.djangoapps.ccx.utils
# & lms.djangoapps.certificates.api
# & lms.djangoapps.discussion.django_comment_client
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.*.*
# cms.djangoapps.export_course_metadata.tasks
# -> openedx.core.djangoapps.schedules.content_highlights
# -> lms.djangoapps.courseware.block_render & lms.djangoapps.courseware.model_data
openedx.core.djangoapps.schedules.content_highlights -> lms.djangoapps.courseware.*
# cms.djangoapps.contentstore.[various]
# -> openedx.core.lib.gating.api
# -> lms.djangoapps.course_blocks.api & lms.djangoapps.courseware.access & lms.djangoapps.grades.api
openedx.core.lib.gating.api -> lms.djangoapps.*.*
# cms.djangoapps.contentstore.[various]
# -> openedx.features.content_type_gating.partitions
# -> openedx.features.discounts.utils
# -> lms.djangoapps.courseware.utils & lms.djangoapps.experiments.models
openedx.features.discounts.utils -> lms.djangoapps.courseware.utils
openedx.features.discounts.utils -> lms.djangoapps.experiments.models
# cms.djangoapps.contentstore.signals.handlers
# -> openedx.core.djangoapps.discussions.tasks
# -> openedx.core.djangoapps.discussions.utils
# -> lms.djangoapps.courseware.access
openedx.core.djangoapps.discussions.utils -> lms.djangoapps.courseware.access
# cms.djangoapps.contentstore.[various]
# -> openedx.features.content_type_gating.partitions
# -> openedx.features.discounts.utils
# -> openedx.features.discounts.applicability
# -> lms.djangoapps.courseware.toggles
# & lms.djangoapps.courseware.utils
# & lms.djangoapps.experiments.models
# & lms.djangoapps.experiments.stable_bucketing
openedx.features.discounts.applicability -> lms.djangoapps.courseware.*
openedx.features.discounts.applicability -> lms.djangoapps.experiments.*
# cms.djangoapps.contentstore.[various]
# -> openedx.core.djangoapps.content.learning_sequences.api
# -> openedx.core.djangoapps.content.learning_sequences.api.outlines
# -> openedx.core.djangoapps.content.learning_sequences.api.permissions
# -> lms.djangoapps.courseware.access
openedx.core.djangoapps.content.learning_sequences.api.permissions -> lms.djangoapps.courseware.access
# cms.djangoapps.contentstore.[various]
# -> openedx.features.content_type_gating.partitions
# -> openedx.features.discounts.utils
# -> openedx.features.discounts.applicability
# -> openedx.features.enterprise_support.utils
openedx.features.enterprise_support.utils -> lms.djangoapps.branding.api
[importlinter:contract:2]
name = Do not depend on non-public API of isolated apps.
type = isolated_apps
isolated_apps =
openedx.core.djangoapps.agreements
openedx.core.djangoapps.bookmarks
openedx.core.djangoapps.content_libraries
openedx.core.djangoapps.olx_rest_api
openedx.core.djangoapps.xblock
allowed_modules =
# Only imports from api.py are allowed elsewhere in the code
# See https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0049-django-app-patterns.html#api-py
api