Skip to content
Merged
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2.11.0

* Added LibraryCollectionKey and LibraryCollectionLocator

# 2.10.0

* Unpin pymongo and upgrade to the latest available version.
Expand Down
2 changes: 1 addition & 1 deletion opaque_keys/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from stevedore.enabled import EnabledExtensionManager
from typing_extensions import Self # For python 3.11 plus, can just use "from typing import Self"

__version__ = '2.10.0'
__version__ = '2.11.0'


class InvalidKeyError(Exception):
Expand Down
21 changes: 21 additions & 0 deletions opaque_keys/edx/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
from __future__ import annotations
import json
from abc import abstractmethod
from typing import TYPE_CHECKING
import warnings

from typing_extensions import Self # For python 3.11 plus, can just use "from typing import Self"

from opaque_keys import OpaqueKey

if TYPE_CHECKING:
from opaque_keys.edx.locator import LibraryLocatorV2


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


class LibraryCollectionKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying a particular Library Collection object.
"""
KEY_TYPE = 'collection_key'
library_key: LibraryLocatorV2
collection_id: str
__slots__ = ()

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


class DefinitionKey(OpaqueKey):
"""
An :class:`opaque_keys.OpaqueKey` identifying an XBlock definition.
Expand Down
58 changes: 57 additions & 1 deletion opaque_keys/edx/locator.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
from typing_extensions import Self # For python 3.11 plus, can just use "from typing import Self"

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

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -1620,3 +1621,58 @@ def html_id(self) -> str:
# HTML5 allows ID values to contain any characters at all other than spaces.
# These key types don't allow spaces either, so no transform is needed.
return str(self)


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

__slots__ = KEY_FIELDS
CHECKED_INIT = False

# Allow usage IDs to contian unicode characters
USAGE_ID_REGEXP = re.compile(r'^[\w\-.]+$', flags=re.UNICODE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the collection id (a number), but I'm unsure if we want to make it stricter here.

Maybe for clarity? What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wait, it's the ID? As in the primary key? We can't use that as part of the key if there's ever an intention to make it importable into a different instance.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At the moment we don't have any other identifier for collections; we'll need to add either a UUID or a slug (or both). If a slug, it only needs to be unique within a library. I prefer slugs for readability. For now the slug can be auto-generated by slugifying the collection name and appending a number until it's unique.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I think a slug is fine, as long as we write it to the database–say, a key field on Collection with a unique constraint on the combo of that and the LearningPackage (like how PublishableEntities work today).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool, let's do that ^ @rpenido . Sorry for not realizing the need for a slug sooner. CC @pomegranited

Comment thread
bradenmacdonald marked this conversation as resolved.
Outdated

def __init__(self, library_key: LibraryLocatorV2, collection_id: str):
"""
Construct a CollectionLocator
"""
if not isinstance(library_key, LibraryLocatorV2):
raise TypeError("library_key must be a LibraryLocatorV2")

self._check_key_string_field("collection_id", collection_id, regexp=self.USAGE_ID_REGEXP)
Comment thread
bradenmacdonald marked this conversation as resolved.
Outdated
super().__init__(
library_key=library_key,
collection_id=collection_id,
)

@property
def org(self) -> str | None: # pragma: no cover
"""
The organization that this collection belongs to.
"""
return self.library_key.org

def _to_string(self) -> str:
"""
Serialize this key as a string
"""
return ":".join((self.library_key.org, self.library_key.slug, self.collection_id))

@classmethod
def _from_string(cls, serialized: str) -> Self:
"""
Instantiate this key from a serialized string
"""
try:
(org, lib_slug, collection_id) = serialized.split(':')
library_key = LibraryLocatorV2(org, lib_slug)
return cls(library_key, collection_id)
except (ValueError, TypeError) as error:
raise InvalidKeyError(cls, serialized) from error
64 changes: 64 additions & 0 deletions opaque_keys/edx/tests/test_collection_locators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
"""
Tests of LibraryCollectionLocator
"""
import ddt
from opaque_keys import InvalidKeyError
from opaque_keys.edx.tests import LocatorBaseTest
from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryLocatorV2


@ddt.ddt
class TestLibraryCollectionLocator(LocatorBaseTest):
"""
Tests of :class:`.LibraryCollectionLocator`
"""
@ddt.data(
"org/lib/id/foo",
"org/lib/id",
"org+lib+id",
"org+lib+",
"org+lib++id@library",
"org+ne@t",
"per%ent+sign",
)
def test_coll_key_from_invalid_string(self, coll_id_str):
with self.assertRaises(InvalidKeyError):
LibraryCollectionLocator.from_string(coll_id_str)

def test_coll_key_constructor(self):
org = 'TestX'
lib = 'LibraryX'
code = 'test-problem-bank'
library_key = LibraryLocatorV2(org=org, slug=lib)
coll_key = LibraryCollectionLocator(library_key=library_key, collection_id=code)
library_key = coll_key.library_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(library_key.org, org)
self.assertEqual(library_key.slug, lib)

def test_coll_key_constructor_bad_ids(self):
library_key = LibraryLocatorV2(org="TestX", slug="lib1")

with self.assertRaises(ValueError):
LibraryCollectionLocator(library_key=library_key, collection_id='usage-!@#{$%^&*}')
with self.assertRaises(TypeError):
LibraryCollectionLocator(library_key=None, collection_id='usage')

def test_coll_key_from_string(self):
org = 'TestX'
lib = 'LibraryX'
code = 'test-problem-bank'
str_key = f"lib-collection:{org}:{lib}:{code}"
coll_key = LibraryCollectionLocator.from_string(str_key)
library_key = coll_key.library_key
self.assertEqual(str(coll_key), str_key)
self.assertEqual(coll_key.org, org)
self.assertEqual(coll_key.collection_id, code)
self.assertEqual(library_key.org, org)
self.assertEqual(library_key.slug, lib)

def test_coll_key_invalid_from_string(self):
with self.assertRaises(InvalidKeyError):
LibraryCollectionLocator.from_string("this-is-a-great-test")
5 changes: 4 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ def get_version(*file_paths):
],
'block_type': [
'block-type-v1 = opaque_keys.edx.block_types:BlockTypeKeyV1',
]
],
'collection_key': [
'lib-collection = opaque_keys.edx.locator:LibraryCollectionLocator',
],
}
)