Skip to content

Commit

Permalink
refactor: Update library_key property of LibraryCollectionKey
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisChV committed Sep 3, 2024
1 parent f2a904f commit ce74081
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 35 deletions.
9 changes: 2 additions & 7 deletions opaque_keys/edx/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ 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
Expand All @@ -107,13 +109,6 @@ def org(self) -> str | None: # pragma: no cover
"""
raise NotImplementedError()

@property
def library_key(self) -> LibraryLocatorV2: # pragma: no cover
"""
Get the key of the library that this collection belongs to.
"""
raise NotImplementedError()


class DefinitionKey(OpaqueKey):
"""
Expand Down
27 changes: 10 additions & 17 deletions opaque_keys/edx/locator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1629,8 +1629,8 @@ class LibraryCollectionLocator(CheckFieldMixin, LibraryCollectionKey):
lib-collection:org:lib:collection-id
"""
CANONICAL_NAMESPACE = 'lib-collection'
KEY_FIELDS = ('lib_key', 'collection_id')
lib_key: LibraryLocatorV2
KEY_FIELDS = ('library_key', 'collection_id')
library_key: LibraryLocatorV2
collection_id: str

__slots__ = KEY_FIELDS
Expand All @@ -1639,38 +1639,31 @@ class LibraryCollectionLocator(CheckFieldMixin, LibraryCollectionKey):
# Allow usage IDs to contian unicode characters
USAGE_ID_REGEXP = re.compile(r'^[\w\-.]+$', flags=re.UNICODE)

def __init__(self, lib_key: LibraryLocatorV2, collection_id: str):
def __init__(self, library_key: LibraryLocatorV2, collection_id: str):
"""
Construct a CollectionLocator
"""
if not isinstance(lib_key, LibraryLocatorV2):
raise TypeError("lib_key must be a LibraryLocatorV2")
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)
super().__init__(
lib_key=lib_key,
library_key=library_key,
collection_id=collection_id,
)

@property
def library_key(self) -> LibraryLocatorV2:
"""
Get the key of the library that this collection belongs to.
"""
return self.lib_key

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

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

@classmethod
def _from_string(cls, serialized: str) -> Self:
Expand All @@ -1679,7 +1672,7 @@ def _from_string(cls, serialized: str) -> Self:
"""
try:
(org, lib_slug, collection_id) = serialized.split(':')
lib_key = LibraryLocatorV2(org, lib_slug)
return cls(lib_key, collection_id)
library_key = LibraryLocatorV2(org, lib_slug)
return cls(library_key, collection_id)
except (ValueError, TypeError) as error:
raise InvalidKeyError(cls, serialized) from error
22 changes: 11 additions & 11 deletions opaque_keys/edx/tests/test_collection_locators.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,35 @@ def test_coll_key_constructor(self):
org = 'TestX'
lib = 'LibraryX'
code = 'test-problem-bank'
lib_key = LibraryLocatorV2(org=org, slug=lib)
coll_key = LibraryCollectionLocator(lib_key=lib_key, collection_id=code)
lib_key = coll_key.library_key
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(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
self.assertEqual(library_key.org, org)
self.assertEqual(library_key.slug, lib)

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

with self.assertRaises(ValueError):
LibraryCollectionLocator(lib_key=lib_key, collection_id='usage-!@#{$%^&*}')
LibraryCollectionLocator(library_key=library_key, collection_id='usage-!@#{$%^&*}')
with self.assertRaises(TypeError):
LibraryCollectionLocator(lib_key=None, collection_id='usage')
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)
lib_key = coll_key.library_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(lib_key.org, org)
self.assertEqual(lib_key.slug, lib)
self.assertEqual(library_key.org, org)
self.assertEqual(library_key.slug, lib)

def test_coll_key_invalid_from_string(self):
with self.assertRaises(InvalidKeyError):
Expand Down

0 comments on commit ce74081

Please sign in to comment.