diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2b6a3b0f4..4822212c7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,16 @@ Change history for XBlock Unreleased ---------- +6.0.0 - 2026-01-20 +------------------ + +* Raise an exception when scope IDs are missing or are not the expected types. In + particular, definition IDs must be DefinitionKey instances and usage IDs must be + UsageKey instances. This has been effectively true within edx-platform (the lone + production client of the XBlock library) for a long time, but explictly + enforcing it will now allow us to add strong type annotations to XBlock in an + upcoming release. + 5.3.0 - 2025-12-19 ------------------ diff --git a/xblock/__init__.py b/xblock/__init__.py index 779a96e05..8f666bd02 100644 --- a/xblock/__init__.py +++ b/xblock/__init__.py @@ -2,4 +2,4 @@ XBlock Courseware Components """ -__version__ = '5.3.0' +__version__ = '6.0.0' diff --git a/xblock/core.py b/xblock/core.py index 7ff362475..171aabcfe 100644 --- a/xblock/core.py +++ b/xblock/core.py @@ -20,7 +20,7 @@ KeyValueMultiSaveError, XBlockSaveError, ) -from xblock.fields import Field, List, Reference, ReferenceList, Scope, String +from xblock.fields import Field, List, Reference, ReferenceList, Scope, String, ScopeIds from xblock.internal import class_lazy from xblock.plugin import Plugin from xblock.validation import Validation @@ -393,6 +393,9 @@ def __init__(self, scope_ids, field_data=None, *, runtime, **kwargs): self._field_data_cache = {} self._dirty_fields = {} + if not isinstance(scope_ids, ScopeIds): + raise TypeError(f"got {scope_ids=}; should be a ScopeIds instance") + scope_ids.validate_types() self.scope_ids = scope_ids super().__init__(**kwargs) @@ -780,9 +783,8 @@ def __init__( self, runtime, field_data=None, - scope_ids=UNSET, - *args, # pylint: disable=keyword-arg-before-vararg - **kwargs + scope_ids=None, + **kwargs, ): """ Arguments: @@ -797,9 +799,6 @@ def __init__( scope_ids (:class:`.ScopeIds`): Identifiers needed to resolve scopes. """ - if scope_ids is UNSET: - raise TypeError('scope_ids are required') - # A cache of the parent block, retrieved from .parent self._parent_block = None self._parent_block_id = None @@ -811,7 +810,7 @@ def __init__( self._parent_block_id = for_parent.scope_ids.usage_id # Provide backwards compatibility for external access through _field_data - super().__init__(runtime=runtime, scope_ids=scope_ids, field_data=field_data, *args, **kwargs) + super().__init__(runtime=runtime, scope_ids=scope_ids, field_data=field_data, **kwargs) def render(self, view, context=None): """Render `view` with this block's runtime and the supplied `context`""" diff --git a/xblock/fields.py b/xblock/fields.py index 6ad3ca1bb..951794d3d 100644 --- a/xblock/fields.py +++ b/xblock/fields.py @@ -3,8 +3,9 @@ **scopes** to associate each field with particular sets of blocks and users. The hosting runtime application decides what actual storage mechanism to use for each scope. - """ +from __future__ import annotations + from collections import namedtuple import copy import datetime @@ -17,6 +18,8 @@ import traceback import warnings +from opaque_keys.edx.keys import UsageKey, DefinitionKey + import dateutil.parser from lxml import etree import pytz @@ -250,6 +253,27 @@ class ScopeIds(namedtuple('ScopeIds', 'user_id block_type def_id usage_id')): """ __slots__ = () + def validate_types(self): + """ + Raise an AssertionError if any of the ids are an unexpected type. + + Originally, these fields were all freely-typed; but in practice, + edx-platform's XBlock runtime would fail if the ids did not match the + types below. In order to make the XBlock library reflect the + edx-platform reality and improve type-safety, we've decided to actually + enforce the types here, per: + https://github.com/openedx/XBlock/issues/708 + """ + if self.user_id is not None: + if not isinstance(self.user_id, (int, str)): + raise TypeError(f"got {self.user_id=}; should be an int, str, or None") + if not isinstance(self.block_type, str): + raise TypeError(f"got {self.block_type=}; should be a str") + if not isinstance(self.def_id, DefinitionKey): + raise TypeError(f"got {self.def_id=}; should be a DefinitionKey") + if not isinstance(self.usage_id, UsageKey): + raise TypeError(f"got {self.usage_id=}; should be a UsageKey") + # Define special reference that can be used as a field's default in field # definition to signal that the field should default to a unique string value diff --git a/xblock/test/test_field_data.py b/xblock/test/test_field_data.py index eaf12281e..a4952f7e1 100644 --- a/xblock/test/test_field_data.py +++ b/xblock/test/test_field_data.py @@ -8,7 +8,7 @@ from xblock.exceptions import InvalidScopeError from xblock.fields import Scope, String from xblock.field_data import SplitFieldData, ReadOnlyFieldData -from xblock.test.tools import TestRuntime +from xblock.test.tools import TestRuntime, make_scope_ids_for_testing class TestingBlock(XBlock): @@ -48,7 +48,7 @@ def setup_method(self): self.runtime = TestRuntime(services={'field-data': self.split}) self.block = TestingBlock( runtime=self.runtime, - scope_ids=Mock(), + scope_ids=make_scope_ids_for_testing(), ) # pylint: enable=attribute-defined-outside-init diff --git a/xblock/test/test_fields.py b/xblock/test/test_fields.py index 1941e7410..630cb9f71 100644 --- a/xblock/test/test_fields.py +++ b/xblock/test/test_fields.py @@ -22,7 +22,7 @@ ScopeIds, Sentinel, UNIQUE_ID, scope_key, Date, Timedelta, RelativeTime, ScoreField, ListScoreField ) from xblock.scorable import Score -from xblock.test.tools import TestRuntime +from xblock.test.tools import TestRuntime, make_scope_ids_for_testing class FieldTest(unittest.TestCase): @@ -41,7 +41,7 @@ class TestBlock(XBlock): field_x = self.FIELD_TO_TEST(enforce_type=enforce_type) runtime = TestRuntime(services={'field-data': DictFieldData({})}) - return TestBlock(runtime, scope_ids=Mock(spec=ScopeIds)) + return TestBlock(runtime, scope_ids=make_scope_ids_for_testing()) def set_and_get_field(self, arg, enforce_type): """ @@ -717,10 +717,11 @@ class TestBlock(XBlock): pref_lst = List(scope=Scope.preferences, name='') user_info_lst = List(scope=Scope.user_info, name='') - sids = ScopeIds(user_id="_bob", - block_type="b.12#ob", - def_id="..", - usage_id="..") + sids = make_scope_ids_for_testing( + user_id="_bob", + block_type="b.12#ob", + block_id="..", + ) field_data = DictFieldData({}) @@ -763,10 +764,11 @@ class TestBlock(XBlock): field_a = String(default=UNIQUE_ID, scope=Scope.settings) field_b = String(default=UNIQUE_ID, scope=Scope.user_state) - sids = ScopeIds(user_id="bob", - block_type="bobs-type", - def_id="definition-id", - usage_id="usage-id") + sids = make_scope_ids_for_testing( + user_id="bob", + block_type="bobs-type", + block_id="usage-id", + ) runtime = TestRuntime(services={'field-data': DictFieldData({})}) block = TestBlock(runtime, DictFieldData({}), sids) @@ -828,7 +830,7 @@ class FieldTester(XBlock): not_timezone_aware = dt.datetime(2015, 1, 1) timezone_aware = dt.datetime(2015, 1, 1, tzinfo=pytz.UTC) runtime = TestRuntime(services={'field-data': DictFieldData({})}) - field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds)) + field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing()) field_tester.incomparable = not_timezone_aware field_tester.incomparable = timezone_aware assert field_tester.incomparable == timezone_aware @@ -853,7 +855,7 @@ class FieldTester(XBlock): original_json = "YYY" runtime = TestRuntime(services={'field-data': DictFieldData({'how_many': original_json})}) - field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds)) + field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing()) # Test that the native value isn't equal to the original json we specified. assert field_tester.how_many != original_json @@ -879,7 +881,7 @@ class FieldTester(XBlock): dict_field = Dict(scope=Scope.settings) runtime = TestRuntime(services={'field-data': DictFieldData({})}) - field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds)) + field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing()) # precondition checks assert len(field_tester._dirty_fields) == 0 diff --git a/xblock/test/tools.py b/xblock/test/tools.py index 98a6e2a6e..4fbf02b5c 100644 --- a/xblock/test/tools.py +++ b/xblock/test/tools.py @@ -2,12 +2,127 @@ Tools for testing XBlocks """ from contextlib import contextmanager +from opaque_keys.edx.keys import UsageKeyV2, LearningContextKey, DefinitionKey from functools import partial +from xblock.fields import ScopeIds import warnings from xblock.runtime import Runtime, MemoryIdManager +def make_scope_ids_for_testing( + user_id=99, + context_slug="myContext", + block_type="myType", + block_id="myId", +): + """ + Make an instance of ScopeIds suitable for testing XBlock. + Any or all parameters can be omitted. + """ + return ScopeIds( + user_id=user_id, + block_type=block_type, + def_id=TestDefinitionKey(block_type, block_id), + usage_id=TestUsageKey(TestContextKey(context_slug), block_type, block_id), + ) + + +class TestDefinitionKey(DefinitionKey): + """ + A simple definition key type for testing XBlock + + When serialized, these keys look like: + td:myType.myId + """ + CANONICAL_NAMESPACE = 'td' # "Test Definition" + KEY_FIELDS = ('block_type', 'block_id') + block_type: str + block_id: str + __slots__ = KEY_FIELDS + CHECKED_INIT = False + + def __init__(self, block_type: str, block_id: str): + super().__init__(block_type=block_type, block_id=block_id) + + def _to_string(self) -> str: + """ + Serialize this key as a string + """ + return f"{self.block_type}.{self.block_id}" + + @classmethod + def _from_string(cls, serialized: str): + """ + Instantiate this key from a serialized string + """ + (block_type, block_id) = serialized.split('.') + return cls(block_type, block_id) + + +class TestContextKey(LearningContextKey): + """ + A simple context key type for testing XBlock + + When serialized, these keys look like: + tc:myContext + """ + CANONICAL_NAMESPACE = 'tc' # "Test Context" + KEY_FIELDS = ('slug',) + slug: str + __slots__ = KEY_FIELDS + CHECKED_INIT = False + + def __init__(self, slug: str): + super().__init__(slug=slug) + + def _to_string(self) -> str: + """ + Serialize this key as a string + """ + return self.slug + + @classmethod + def _from_string(cls, serialized: str): + """ + Instantiate this key from a serialized string + """ + return cls(serialized) + + +class TestUsageKey(UsageKeyV2): + """ + A simple usage key type for testing XBlock + + When serialized, these keys look like: + tu:myContext.myType.myId + """ + CANONICAL_NAMESPACE = 'tu' # "Test Usage" + KEY_FIELDS = ('context_key', 'block_type', 'block_id') + context_key: TestContextKey + block_type: str + block_id: str + __slots__ = KEY_FIELDS + CHECKED_INIT = False + + def __init__(self, context_key: TestContextKey, block_type: str, block_id: str): + super().__init__(context_key=context_key, block_type=block_type, block_id=block_id) + + def _to_string(self) -> str: + """ + Serialize this key as a string + """ + return ".".join((self.context_key.slug, self.block_type, self.block_id)) + + @classmethod + def _from_string(cls, serialized: str): + """ + Instantiate this key from a serialized string + """ + (context_slug, block_type, block_id) = serialized.split('.') + return cls(TestContextKey(context_slug), block_type, block_id) + + def blocks_are_equivalent(block1, block2): """Compare two blocks for equivalence. """