From 168c373f4adafe7c81f707baef3a971f7404acd7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 30 Aug 2024 23:54:03 -0500 Subject: [PATCH] refactor: LibraryCollectionLocator to not store org and lib slug and obtain that data from library_key --- opaque_keys/edx/keys.py | 15 +++------ opaque_keys/edx/locator.py | 32 +++++++++++-------- .../edx/tests/test_collection_locators.py | 12 +++---- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/opaque_keys/edx/keys.py b/opaque_keys/edx/keys.py index ec87cb2..c9df7f6 100644 --- a/opaque_keys/edx/keys.py +++ b/opaque_keys/edx/keys.py @@ -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 """ @@ -97,7 +101,6 @@ class LibraryCollectionKey(OpaqueKey): __slots__ = () @property - @abstractmethod def org(self) -> str | None: # pragma: no cover """ The organization that this collection belongs to. @@ -105,15 +108,7 @@ def org(self) -> str | None: # pragma: no cover raise NotImplementedError() @property - @abstractmethod - def lib(self) -> str | None: # pragma: no cover - """ - The lib for this collection. - """ - raise NotImplementedError() - - @property - def library_key(self) -> LearningContextKey: # pragma: no cover + def library_key(self) -> LibraryLocatorV2: # pragma: no cover """ Get the key of the library that this collection belongs to. """ diff --git a/opaque_keys/edx/locator.py b/opaque_keys/edx/locator.py index 968d64c..6dacb86 100644 --- a/opaque_keys/edx/locator.py +++ b/opaque_keys/edx/locator.py @@ -1629,11 +1629,9 @@ class LibraryCollectionLocator(CheckFieldMixin, LibraryCollectionKey): lib-collection:org:lib:collection-id """ CANONICAL_NAMESPACE = 'lib-collection' - KEY_FIELDS = ('org', 'lib', 'usage_id') - org: str - lib: str + KEY_FIELDS = ('lib_key', 'collection_id') lib_key: LibraryLocatorV2 - usage_id: str + collection_id: str __slots__ = KEY_FIELDS CHECKED_INIT = False @@ -1641,30 +1639,38 @@ 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, usage_id: str): + def __init__(self, lib_key: LibraryLocatorV2, collection_id: str): """ Construct a CollectionLocator """ if not isinstance(lib_key, LibraryLocatorV2): raise TypeError("lib_key must be a LibraryLocatorV2") - self._check_key_string_field("usage_id", usage_id, regexp=self.USAGE_ID_REGEXP) + self._check_key_string_field("collection_id", collection_id, regexp=self.USAGE_ID_REGEXP) super().__init__( - org=lib_key.org, - lib=lib_key.slug, lib_key=lib_key, - usage_id=usage_id, + 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 + def _to_string(self) -> str: """ Serialize this key as a string """ - return ":".join((self.org, self.lib, self.usage_id)) + return ":".join((self.lib_key.org, self.lib_key.slug, self.collection_id)) @classmethod def _from_string(cls, serialized: str) -> Self: @@ -1672,8 +1678,8 @@ def _from_string(cls, serialized: str) -> Self: Instantiate this key from a serialized string """ try: - (org, lib, usage_id) = serialized.split(':') - lib_key = LibraryLocatorV2(org, lib) - return cls(lib_key, usage_id) + (org, lib_slug, collection_id) = serialized.split(':') + lib_key = LibraryLocatorV2(org, lib_slug) + return cls(lib_key, collection_id) except (ValueError, TypeError) as error: raise InvalidKeyError(cls, serialized) from error diff --git a/opaque_keys/edx/tests/test_collection_locators.py b/opaque_keys/edx/tests/test_collection_locators.py index 5946886..5e04229 100644 --- a/opaque_keys/edx/tests/test_collection_locators.py +++ b/opaque_keys/edx/tests/test_collection_locators.py @@ -30,12 +30,11 @@ def test_coll_key_constructor(self): lib = 'LibraryX' code = 'test-problem-bank' lib_key = LibraryLocatorV2(org=org, slug=lib) - coll_key = LibraryCollectionLocator(lib_key=lib_key, usage_id=code) + coll_key = LibraryCollectionLocator(lib_key=lib_key, collection_id=code) lib_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.lib, lib) - self.assertEqual(coll_key.usage_id, code) + self.assertEqual(coll_key.collection_id, code) self.assertEqual(lib_key.org, org) self.assertEqual(lib_key.slug, lib) @@ -43,9 +42,9 @@ def test_coll_key_constructor_bad_ids(self): lib_key = LibraryLocatorV2(org="TestX", slug="lib1") with self.assertRaises(ValueError): - LibraryCollectionLocator(lib_key=lib_key, usage_id='usage-!@#{$%^&*}') + LibraryCollectionLocator(lib_key=lib_key, collection_id='usage-!@#{$%^&*}') with self.assertRaises(TypeError): - LibraryCollectionLocator(lib_key=None, usage_id='usage') + LibraryCollectionLocator(lib_key=None, collection_id='usage') def test_coll_key_from_string(self): org = 'TestX' @@ -56,8 +55,7 @@ def test_coll_key_from_string(self): lib_key = coll_key.library_key self.assertEqual(str(coll_key), str_key) self.assertEqual(coll_key.org, org) - self.assertEqual(coll_key.lib, lib) - self.assertEqual(coll_key.usage_id, code) + self.assertEqual(coll_key.collection_id, code) self.assertEqual(lib_key.org, org) self.assertEqual(lib_key.slug, lib)