Skip to content

Consistency fix (breaking): LibraryItemKey is replaced by CollectionKey and ContainerKey #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bradenmacdonald merged 2 commits into from
Apr 23, 2025
Merged

Consistency fix (breaking): LibraryItemKey is replaced by CollectionKey and ContainerKey #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8574864
feat!: LibraryItemKey is replaced by CollectionKey and ContainerKey
bradenmacdonald Apr 23, 2025
ce976fb
chore: increase coverage
bradenmacdonald Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 3.0.0

* Breaking: LibraryItemKey is replaced by CollectionKey and ContainerKey

# 2.13.0

* Breaking change to the new LibraryContainerLocator and
Expand Down
2 changes: 1 addition & 1 deletion opaque_keys/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from stevedore.enabled import EnabledExtensionManager

__version__ = '2.13.0'
__version__ = '3.0.0'


class InvalidKeyError(Exception):
Expand Down
26 changes: 24 additions & 2 deletions opaque_keys/edx/django/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
If Django is unavailable, none of the classes below will work as intended.
"""
from __future__ import annotations

import logging
import warnings

Expand All @@ -17,8 +18,7 @@
IsNull = object

from opaque_keys import OpaqueKey
from opaque_keys.edx.keys import BlockTypeKey, CourseKey, LearningContextKey, UsageKey

from opaque_keys.edx.keys import BlockTypeKey, CourseKey, LearningContextKey, ContainerKey, CollectionKey, UsageKey

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -224,6 +224,28 @@ class UsageKeyField(OpaqueKeyField):
_pyi_private_get_type: UsageKey | None


class ContainerKeyField(OpaqueKeyField):
"""
A django Field that stores a ContainerKey object as a string.
"""
description = "A Location object, saved to the DB in the form of a string"
KEY_CLASS = ContainerKey
# Declare the field types for the django-stubs mypy type hint plugin:
_pyi_private_set_type: ContainerKey | str | None
_pyi_private_get_type: ContainerKey | None


class CollectionKeyField(OpaqueKeyField):
"""
A django Field that stores a CollectionKey object as a string.
"""
description = "A Location object, saved to the DB in the form of a string"
KEY_CLASS = CollectionKey
# Declare the field types for the django-stubs mypy type hint plugin:
_pyi_private_set_type: CollectionKey | str | None
_pyi_private_get_type: CollectionKey | None


class LocationKeyField(UsageKeyField):
"""
A django Field that stores a UsageKey object as a string.
Expand Down
9 changes: 8 additions & 1 deletion opaque_keys/edx/django/tests/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
Model = object

from opaque_keys.edx.django.models import (
BlockTypeKeyField, CourseKeyField, CreatorMixin, UsageKeyField
BlockTypeKeyField,
CollectionKeyField,
ContainerKeyField,
CourseKeyField,
CreatorMixin,
UsageKeyField,
)


Expand Down Expand Up @@ -64,3 +69,5 @@ class ComplexModel(Model):
course_key = CourseKeyField(max_length=255, validators=[is_edx])
block_type_key = BlockTypeKeyField(max_length=255, blank=True)
usage_key = UsageKeyField(max_length=255, blank=False)
collection_key = CollectionKeyField(max_length=255, blank=False)
container_key = ContainerKeyField(max_length=255, blank=False)
8 changes: 6 additions & 2 deletions opaque_keys/edx/django/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import pytest

from opaque_keys.edx.django.models import OpaqueKeyField, UsageKeyField
from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.keys import CollectionKey, ContainerKey, CourseKey, UsageKey

from .models import ComplexModel, Container, ExampleModel

Expand Down Expand Up @@ -68,10 +68,14 @@ def setUp(self):
super().setUp()
self.course_key = CourseKey.from_string('course-v1:edX+FUN101x+3T2017')
self.usage_key = UsageKey.from_string('block-v1:edX+FUN101x+3T2017+type@html+block@12345678')
self.collection_key = CollectionKey.from_string('lib-collection:TestX:LibraryX:test-problem-bank')
self.container_key = ContainerKey.from_string('lct:TestX:LibraryX:unit:test-container')
self.model = ComplexModel(
id='foobar',
course_key=self.course_key,
usage_key=self.usage_key
usage_key=self.usage_key,
collection_key=self.collection_key,
container_key=self.container_key,
)
self.model.save()

Expand Down
63 changes: 35 additions & 28 deletions opaque_keys/edx/keys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,11 @@
from __future__ import annotations
import json
from abc import abstractmethod
from typing import TYPE_CHECKING, Self
from typing import Self
import warnings

from typing_extensions import deprecated # For python 3.13+ can use 'from warnings import deprecated'

from opaque_keys import OpaqueKey

if TYPE_CHECKING:
from opaque_keys.edx.locator import LibraryLocatorV2
Comment on lines -14 to -15
Copy link
Contributor Author

@bradenmacdonald bradenmacdonald Apr 23, 2025

Choose a reason for hiding this comment

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

This refactor allows us to get rid of this messy import of a locator (subclass) in the keys (parent class) file.



class LearningContextKey(OpaqueKey): # pylint: disable=abstract-method
"""
Expand Down Expand Up @@ -93,28 +88,6 @@ def make_asset_key(self, asset_type: str, path: str) -> AssetKey: # pragma: no
raise NotImplementedError()


class LibraryItemKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying a particular item in a library.
"""
KEY_TYPE = 'library_item_key'
lib_key: LibraryLocatorV2
__slots__ = ()

@property
@abstractmethod
def org(self) -> str | None: # pragma: no cover
"""
The organization that this object belongs to.
"""
raise NotImplementedError()

@property
@deprecated("Use lib_key instead")
def library_key(self):
return self.lib_key


class DefinitionKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying an XBlock definition.
Expand Down Expand Up @@ -281,6 +254,40 @@ def map_into_course(self, course_key: CourseKey) -> Self:
raise ValueError("Cannot use map_into_course like that with this key type.")


class ContainerKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying a container (a non-XBlock
structure like a unit, section, or subsection).
"""
KEY_TYPE = 'container_key'
__slots__ = ()

@property
@abstractmethod
def context_key(self) -> LearningContextKey: # pragma: no cover
"""
Get the learning context key (LearningContextKey) for this container.
"""
raise NotImplementedError()


class CollectionKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying a collection (a group of
content, mostly used in libraries to organize components/containers).
"""
KEY_TYPE = 'collection_key'
__slots__ = ()

@property
@abstractmethod
def context_key(self) -> LearningContextKey: # pragma: no cover
"""
Get the learning context key (LearningContextKey) for this collection.
"""
raise NotImplementedError()


class AsideDefinitionKey(DefinitionKey):
"""
A definition key for an aside.
Expand Down
16 changes: 13 additions & 3 deletions opaque_keys/edx/locator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from opaque_keys import OpaqueKey, InvalidKeyError
from opaque_keys.edx.keys import AssetKey, CourseKey, DefinitionKey, \
LearningContextKey, UsageKey, UsageKeyV2, LibraryItemKey
LearningContextKey, UsageKey, UsageKeyV2, ContainerKey, CollectionKey

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -1622,13 +1622,14 @@ def html_id(self) -> str:
return str(self)


class LibraryCollectionLocator(CheckFieldMixin, LibraryItemKey):
class LibraryCollectionLocator(CheckFieldMixin, CollectionKey):
"""
When serialized, these keys look like:
lib-collection:org:lib:collection-id
"""
CANONICAL_NAMESPACE = 'lib-collection'
KEY_FIELDS = ('lib_key', 'collection_id')
lib_key: LibraryLocatorV2
collection_id: str

__slots__ = KEY_FIELDS
Expand Down Expand Up @@ -1675,14 +1676,19 @@ def _from_string(cls, serialized: str) -> Self:
except (ValueError, TypeError) as error:
raise InvalidKeyError(cls, serialized) from error

@property
def context_key(self) -> LibraryLocatorV2:
return self.lib_key


class LibraryContainerLocator(CheckFieldMixin, LibraryItemKey):
class LibraryContainerLocator(CheckFieldMixin, ContainerKey):
"""
When serialized, these keys look like:
lct:org:lib:ct-type:ct-id
"""
CANONICAL_NAMESPACE = 'lct' # "Library Container"
KEY_FIELDS = ('lib_key', 'container_type', 'container_id')
lib_key: LibraryLocatorV2
container_type: str
container_id: str

Expand Down Expand Up @@ -1714,6 +1720,10 @@ def org(self) -> str | None: # pragma: no cover
"""
return self.lib_key.org

@property
def context_key(self) -> LibraryLocatorV2:
return self.lib_key

def _to_string(self) -> str:
"""
Serialize this key as a string
Expand Down
27 changes: 17 additions & 10 deletions opaque_keys/edx/tests/test_collection_locators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ddt
from opaque_keys import InvalidKeyError
from opaque_keys.edx.tests import LocatorBaseTest
from opaque_keys.edx.keys import CollectionKey
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryLocatorV2


Expand Down Expand Up @@ -32,11 +33,12 @@ def test_coll_key_constructor(self):
lib_key = LibraryLocatorV2(org=org, slug=lib)
coll_key = LibraryCollectionLocator(lib_key=lib_key, collection_id=code)
lib_key = coll_key.lib_key
self.assertEqual(str(coll_key), "lib-collection:TestX:LibraryX:test-problem-bank")
self.assertEqual(coll_key.org, org)
self.assertEqual(coll_key.collection_id, code)
self.assertEqual(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
assert str(coll_key) == "lib-collection:TestX:LibraryX:test-problem-bank"
assert coll_key.org == org
assert coll_key.collection_id == code
assert lib_key.org == org
assert lib_key.slug == lib
assert isinstance(coll_key, CollectionKey)

def test_coll_key_constructor_bad_ids(self):
lib_key = LibraryLocatorV2(org="TestX", slug="lib1")
Expand All @@ -52,13 +54,18 @@ def test_coll_key_from_string(self):
code = 'test-problem-bank'
str_key = f"lib-collection:{org}:{lib}:{code}"
coll_key = LibraryCollectionLocator.from_string(str_key)
assert coll_key == CollectionKey.from_string(str_key)
assert str(coll_key) == str_key
assert coll_key.org == org
assert coll_key.collection_id == code
lib_key = coll_key.lib_key
self.assertEqual(str(coll_key), str_key)
self.assertEqual(coll_key.org, org)
self.assertEqual(coll_key.collection_id, code)
self.assertEqual(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
assert isinstance(lib_key, LibraryLocatorV2)
assert lib_key.org == org
assert lib_key.slug == lib
assert coll_key.context_key == lib_key

def test_coll_key_invalid_from_string(self):
with self.assertRaises(InvalidKeyError):
LibraryCollectionLocator.from_string("this-is-a-great-test")
with self.assertRaises(InvalidKeyError):
LibraryCollectionLocator.from_string("lib-collection:TestX:LibraryX:test:too:many:colons")
29 changes: 17 additions & 12 deletions opaque_keys/edx/tests/test_container_locators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ddt
from opaque_keys import InvalidKeyError
from opaque_keys.edx.tests import LocatorBaseTest
from opaque_keys.edx.keys import ContainerKey
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2


Expand Down Expand Up @@ -37,12 +38,13 @@ def test_key_constructor(self):
container_id=container_id,
)
lib_key = container_key.lib_key
self.assertEqual(str(container_key), "lct:TestX:LibraryX:unit:test-container")
self.assertEqual(container_key.org, org)
self.assertEqual(container_key.container_type, container_type)
self.assertEqual(container_key.container_id, container_id)
self.assertEqual(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
assert str(container_key) == "lct:TestX:LibraryX:unit:test-container"
assert container_key.org == org
assert container_key.container_type == container_type
assert container_key.container_id == container_id
assert lib_key.org == org
assert lib_key.slug == lib
assert isinstance(container_key, ContainerKey)

def test_key_constructor_bad_ids(self):
lib_key = LibraryLocatorV2(org="TestX", slug="lib1")
Expand All @@ -66,10 +68,13 @@ def test_key_from_string(self):
container_id = 'test-container'
str_key = f"lct:{org}:{lib}:{container_type}:{container_id}"
container_key = LibraryContainerLocator.from_string(str_key)
assert container_key == ContainerKey.from_string(str_key)
assert str(container_key) == str_key
assert container_key.org == org
assert container_key.container_type == container_type
assert container_key.container_id == container_id
lib_key = container_key.lib_key
self.assertEqual(str(container_key), str_key)
self.assertEqual(container_key.org, org)
self.assertEqual(container_key.container_type, container_type)
self.assertEqual(container_key.container_id, container_id)
self.assertEqual(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
assert isinstance(lib_key, LibraryLocatorV2)
assert lib_key.org == org
assert lib_key.slug == lib
assert container_key.context_key == lib_key
4 changes: 3 additions & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,10 @@ def get_version(*file_paths):
'block_type': [
'block-type-v1 = opaque_keys.edx.block_types:BlockTypeKeyV1',
],
'library_item_key': [
'collection_key': [
'lib-collection = opaque_keys.edx.locator:LibraryCollectionLocator',
],
'container_key': [
'lct = opaque_keys.edx.locator:LibraryContainerLocator',
],
}
Expand Down