Skip to content

Commit d4e5e83

Browse files
committed
feat!: Assume that scope IDs are opaque key objects
BREAKING CHANGE: 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. Bump version to 6.0.0. Closes: #708
1 parent e1e7ef6 commit d4e5e83

7 files changed

Lines changed: 175 additions & 25 deletions

File tree

CHANGELOG.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ Change history for XBlock
55
Unreleased
66
----------
77

8+
6.0.0 - 2026-01-20
9+
------------------
10+
11+
* Raise an exception when scope IDs are missing or are not the expected types. In
12+
particular, definition IDs must be DefinitionKey instances and usage IDs must be
13+
UsageKey instances. This has been effectively true within edx-platform (the lone
14+
production client of the XBlock library) for a long time, but explictly
15+
enforcing it will now allow us to add strong type annotations to XBlock in an
16+
upcoming release.
17+
818
5.3.0 - 2025-12-19
919
------------------
1020

xblock/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
XBlock Courseware Components
33
"""
44

5-
__version__ = '5.3.0'
5+
__version__ = '6.0.0'

xblock/core.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
KeyValueMultiSaveError,
2121
XBlockSaveError,
2222
)
23-
from xblock.fields import Field, List, Reference, ReferenceList, Scope, String
23+
from xblock.fields import Field, List, Reference, ReferenceList, Scope, String, ScopeIds
2424
from xblock.internal import class_lazy
2525
from xblock.plugin import Plugin
2626
from xblock.validation import Validation
@@ -393,6 +393,9 @@ def __init__(self, scope_ids, field_data=None, *, runtime, **kwargs):
393393

394394
self._field_data_cache = {}
395395
self._dirty_fields = {}
396+
if not isinstance(scope_ids, ScopeIds):
397+
raise TypeError(f"got {scope_ids=}; should be a ScopeIds instance")
398+
scope_ids.validate_types()
396399
self.scope_ids = scope_ids
397400

398401
super().__init__(**kwargs)
@@ -780,9 +783,8 @@ def __init__(
780783
self,
781784
runtime,
782785
field_data=None,
783-
scope_ids=UNSET,
784-
*args, # pylint: disable=keyword-arg-before-vararg
785-
**kwargs
786+
scope_ids=None,
787+
**kwargs,
786788
):
787789
"""
788790
Arguments:
@@ -797,9 +799,6 @@ def __init__(
797799
scope_ids (:class:`.ScopeIds`): Identifiers needed to resolve
798800
scopes.
799801
"""
800-
if scope_ids is UNSET:
801-
raise TypeError('scope_ids are required')
802-
803802
# A cache of the parent block, retrieved from .parent
804803
self._parent_block = None
805804
self._parent_block_id = None
@@ -811,7 +810,7 @@ def __init__(
811810
self._parent_block_id = for_parent.scope_ids.usage_id
812811

813812
# Provide backwards compatibility for external access through _field_data
814-
super().__init__(runtime=runtime, scope_ids=scope_ids, field_data=field_data, *args, **kwargs)
813+
super().__init__(runtime=runtime, scope_ids=scope_ids, field_data=field_data, **kwargs)
815814

816815
def render(self, view, context=None):
817816
"""Render `view` with this block's runtime and the supplied `context`"""

xblock/fields.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
**scopes** to associate each field with particular sets of blocks and users.
44
The hosting runtime application decides what actual storage mechanism to use
55
for each scope.
6-
76
"""
7+
from __future__ import annotations
8+
89
from collections import namedtuple
910
import copy
1011
import datetime
@@ -17,6 +18,8 @@
1718
import traceback
1819
import warnings
1920

21+
from opaque_keys.edx.keys import UsageKey, DefinitionKey
22+
2023
import dateutil.parser
2124
from lxml import etree
2225
import pytz
@@ -250,6 +253,27 @@ class ScopeIds(namedtuple('ScopeIds', 'user_id block_type def_id usage_id')):
250253
"""
251254
__slots__ = ()
252255

256+
def validate_types(self):
257+
"""
258+
Raise an AssertionError if any of the ids are an unexpected type.
259+
260+
Originally, these fields were all freely-typed; but in practice,
261+
edx-platform's XBlock runtime would fail if the ids did not match the
262+
types below. In order to make the XBlock library reflect the
263+
edx-platform reality and improve type-safety, we've decided to actually
264+
enforce the types here, per:
265+
https://github.com/openedx/XBlock/issues/708
266+
"""
267+
if self.user_id is not None:
268+
if not isinstance(self.user_id, (int, str)):
269+
raise TypeError(f"got {self.user_id=}; should be an int, str, or None")
270+
if not isinstance(self.block_type, str):
271+
raise TypeError(f"got {self.block_type=}; should be a str")
272+
if not isinstance(self.def_id, DefinitionKey):
273+
raise TypeError(f"got {self.def_id=}; should be a DefinitionKey")
274+
if not isinstance(self.usage_id, UsageKey):
275+
raise TypeError(f"got {self.usage_id=}; should be a UsageKey")
276+
253277

254278
# Define special reference that can be used as a field's default in field
255279
# definition to signal that the field should default to a unique string value

xblock/test/test_field_data.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from xblock.exceptions import InvalidScopeError
99
from xblock.fields import Scope, String
1010
from xblock.field_data import SplitFieldData, ReadOnlyFieldData
11-
from xblock.test.tools import TestRuntime
11+
from xblock.test.tools import TestRuntime, make_scope_ids_for_testing
1212

1313

1414
class TestingBlock(XBlock):
@@ -48,7 +48,7 @@ def setup_method(self):
4848
self.runtime = TestRuntime(services={'field-data': self.split})
4949
self.block = TestingBlock(
5050
runtime=self.runtime,
51-
scope_ids=Mock(),
51+
scope_ids=make_scope_ids_for_testing(),
5252
)
5353
# pylint: enable=attribute-defined-outside-init
5454

xblock/test/test_fields.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
ScopeIds, Sentinel, UNIQUE_ID, scope_key, Date, Timedelta, RelativeTime, ScoreField, ListScoreField
2323
)
2424
from xblock.scorable import Score
25-
from xblock.test.tools import TestRuntime
25+
from xblock.test.tools import TestRuntime, make_scope_ids_for_testing
2626

2727

2828
class FieldTest(unittest.TestCase):
@@ -41,7 +41,7 @@ class TestBlock(XBlock):
4141
field_x = self.FIELD_TO_TEST(enforce_type=enforce_type)
4242

4343
runtime = TestRuntime(services={'field-data': DictFieldData({})})
44-
return TestBlock(runtime, scope_ids=Mock(spec=ScopeIds))
44+
return TestBlock(runtime, scope_ids=make_scope_ids_for_testing())
4545

4646
def set_and_get_field(self, arg, enforce_type):
4747
"""
@@ -717,10 +717,11 @@ class TestBlock(XBlock):
717717
pref_lst = List(scope=Scope.preferences, name='')
718718
user_info_lst = List(scope=Scope.user_info, name='')
719719

720-
sids = ScopeIds(user_id="_bob",
721-
block_type="b.12#ob",
722-
def_id="..",
723-
usage_id="..")
720+
sids = make_scope_ids_for_testing(
721+
user_id="_bob",
722+
block_type="b.12#ob",
723+
block_id="..",
724+
)
724725

725726
field_data = DictFieldData({})
726727

@@ -763,10 +764,11 @@ class TestBlock(XBlock):
763764
field_a = String(default=UNIQUE_ID, scope=Scope.settings)
764765
field_b = String(default=UNIQUE_ID, scope=Scope.user_state)
765766

766-
sids = ScopeIds(user_id="bob",
767-
block_type="bobs-type",
768-
def_id="definition-id",
769-
usage_id="usage-id")
767+
sids = make_scope_ids_for_testing(
768+
user_id="bob",
769+
block_type="bobs-type",
770+
block_id="usage-id",
771+
)
770772

771773
runtime = TestRuntime(services={'field-data': DictFieldData({})})
772774
block = TestBlock(runtime, DictFieldData({}), sids)
@@ -828,7 +830,7 @@ class FieldTester(XBlock):
828830
not_timezone_aware = dt.datetime(2015, 1, 1)
829831
timezone_aware = dt.datetime(2015, 1, 1, tzinfo=pytz.UTC)
830832
runtime = TestRuntime(services={'field-data': DictFieldData({})})
831-
field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds))
833+
field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing())
832834
field_tester.incomparable = not_timezone_aware
833835
field_tester.incomparable = timezone_aware
834836
assert field_tester.incomparable == timezone_aware
@@ -853,7 +855,7 @@ class FieldTester(XBlock):
853855

854856
original_json = "YYY"
855857
runtime = TestRuntime(services={'field-data': DictFieldData({'how_many': original_json})})
856-
field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds))
858+
field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing())
857859

858860
# Test that the native value isn't equal to the original json we specified.
859861
assert field_tester.how_many != original_json
@@ -879,7 +881,7 @@ class FieldTester(XBlock):
879881
dict_field = Dict(scope=Scope.settings)
880882

881883
runtime = TestRuntime(services={'field-data': DictFieldData({})})
882-
field_tester = FieldTester(runtime, scope_ids=Mock(spec=ScopeIds))
884+
field_tester = FieldTester(runtime, scope_ids=make_scope_ids_for_testing())
883885

884886
# precondition checks
885887
assert len(field_tester._dirty_fields) == 0

xblock/test/tools.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,127 @@
22
Tools for testing XBlocks
33
"""
44
from contextlib import contextmanager
5+
from opaque_keys.edx.keys import UsageKeyV2, LearningContextKey, DefinitionKey
56
from functools import partial
7+
from xblock.fields import ScopeIds
68
import warnings
79

810
from xblock.runtime import Runtime, MemoryIdManager
911

1012

13+
def make_scope_ids_for_testing(
14+
user_id=99,
15+
context_slug="myContext",
16+
block_type="myType",
17+
block_id="myId",
18+
):
19+
"""
20+
Make an instance of ScopeIds suitable for testing XBlock.
21+
Any or all parameters can be omitted.
22+
"""
23+
return ScopeIds(
24+
user_id=user_id,
25+
block_type=block_type,
26+
def_id=TestDefinitionKey(block_type, block_id),
27+
usage_id=TestUsageKey(TestContextKey(context_slug), block_type, block_id),
28+
)
29+
30+
31+
class TestDefinitionKey(DefinitionKey):
32+
"""
33+
A simple definition key type for testing XBlock
34+
35+
When serialized, these keys look like:
36+
td:myType.myId
37+
"""
38+
CANONICAL_NAMESPACE = 'td' # "Test Definition"
39+
KEY_FIELDS = ('block_type', 'block_id')
40+
block_type: str
41+
block_id: str
42+
__slots__ = KEY_FIELDS
43+
CHECKED_INIT = False
44+
45+
def __init__(self, block_type: str, block_id: str):
46+
super().__init__(block_type=block_type, block_id=block_id)
47+
48+
def _to_string(self) -> str:
49+
"""
50+
Serialize this key as a string
51+
"""
52+
return f"{self.block_type}.{self.block_id}"
53+
54+
@classmethod
55+
def _from_string(cls, serialized: str):
56+
"""
57+
Instantiate this key from a serialized string
58+
"""
59+
(block_type, block_id) = serialized.split('.')
60+
return cls(block_type, block_id)
61+
62+
63+
class TestContextKey(LearningContextKey):
64+
"""
65+
A simple context key type for testing XBlock
66+
67+
When serialized, these keys look like:
68+
tc:myContext
69+
"""
70+
CANONICAL_NAMESPACE = 'tc' # "Test Context"
71+
KEY_FIELDS = ('slug',)
72+
slug: str
73+
__slots__ = KEY_FIELDS
74+
CHECKED_INIT = False
75+
76+
def __init__(self, slug: str):
77+
super().__init__(slug=slug)
78+
79+
def _to_string(self) -> str:
80+
"""
81+
Serialize this key as a string
82+
"""
83+
return self.slug
84+
85+
@classmethod
86+
def _from_string(cls, serialized: str):
87+
"""
88+
Instantiate this key from a serialized string
89+
"""
90+
return cls(serialized)
91+
92+
93+
class TestUsageKey(UsageKeyV2):
94+
"""
95+
A simple usage key type for testing XBlock
96+
97+
When serialized, these keys look like:
98+
tu:myContext.myType.myId
99+
"""
100+
CANONICAL_NAMESPACE = 'tu' # "Test Usage"
101+
KEY_FIELDS = ('context_key', 'block_type', 'block_id')
102+
context_key: TestContextKey
103+
block_type: str
104+
block_id: str
105+
__slots__ = KEY_FIELDS
106+
CHECKED_INIT = False
107+
108+
def __init__(self, context_key: TestContextKey, block_type: str, block_id: str):
109+
super().__init__(context_key=context_key, block_type=block_type, block_id=block_id)
110+
111+
def _to_string(self) -> str:
112+
"""
113+
Serialize this key as a string
114+
"""
115+
return ".".join((self.context_key.slug, self.block_type, self.block_id))
116+
117+
@classmethod
118+
def _from_string(cls, serialized: str):
119+
"""
120+
Instantiate this key from a serialized string
121+
"""
122+
(context_slug, block_type, block_id) = serialized.split('.')
123+
return cls(TestContextKey(context_slug), block_type, block_id)
124+
125+
11126
def blocks_are_equivalent(block1, block2):
12127
"""Compare two blocks for equivalence.
13128
"""

0 commit comments

Comments
 (0)