From 7384d5854cce45fb9da8d99dd72ac9090923c0ca Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 28 Oct 2025 11:16:12 -0500 Subject: [PATCH 1/5] feat: implement get_object method in ContentLibraryData --- openedx_authz/api/data.py | 51 ++++++++++-- openedx_authz/tests/api/test_data.py | 114 +++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 7 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index effc6ad2..e1d100f3 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -1,9 +1,11 @@ """Data classes and enums for representing roles, permissions, and policies.""" +from __future__ import annotations + import re from abc import abstractmethod from enum import Enum -from typing import ClassVar, Literal, Type +from typing import Any, ClassVar, Literal, Type from attrs import define from opaque_keys import InvalidKeyError @@ -319,6 +321,20 @@ def validate_external_key(cls, _: str) -> bool: """ return True + @abstractmethod + def get_object(self) -> Any | None: + """Retrieve the underlying domain object that this scope represents. + + This method fetches the actual Open edX object (e.g., ContentLibrary, Organization) + associated with this scope's external_key. Subclasses should implement this to return + their specific object types. + + Returns: + Any | None: The domain object associated with this scope, or None if the object + does not exist or cannot be retrieved. + """ + raise NotImplementedError("Subclasses must implement get_object method.") + @abstractmethod def exists(self) -> bool: """Check if the scope exists. @@ -382,18 +398,39 @@ def validate_external_key(cls, external_key: str) -> bool: except InvalidKeyError: return False + def get_object(self) -> ContentLibrary | None: + """Retrieve the ContentLibrary instance associated with this scope. + + This method converts the library_id to a LibraryLocatorV2 key and queries the + database to fetch the corresponding ContentLibrary object. + + Returns: + ContentLibrary | None: The ContentLibrary instance if found in the database, + or None if the library does not exist or has an invalid key format. + + Examples: + >>> library_scope = ContentLibraryData(external_key='lib:DemoX:CSPROB') + >>> library_obj = library_scope.get_object() # ContentLibrary object + """ + try: + library_key = LibraryLocatorV2.from_string(self.library_id) + library_obj = ContentLibrary.objects.get_by_key(library_key=library_key) + # Validate canonical key: get_by_key is case-insensitive, but we require exact match + # This ensures authorization uses canonical library IDs consistently + if library_obj.library_key != library_key: + raise ContentLibrary.DoesNotExist + except (InvalidKeyError, ContentLibrary.DoesNotExist): + return None + + return library_obj + def exists(self) -> bool: """Check if the content library exists. Returns: bool: True if the content library exists, False otherwise. """ - try: - library_key = LibraryLocatorV2.from_string(self.library_id) - ContentLibrary.objects.get_by_key(library_key=library_key) - return True - except ContentLibrary.DoesNotExist: - return False + return self.get_object() is not None def __str__(self): """Human readable string representation of the content library.""" diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 8a9ca928..55e42260 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -1,7 +1,11 @@ """Test data for the authorization API.""" +from unittest.mock import Mock, patch + from ddt import data, ddt, unpack from django.test import TestCase +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz.api.data import ( ActionData, @@ -507,3 +511,113 @@ def test_role_assignment_data_repr(self): expected_repr = "user^john_doe => [role^instructor, role^library_admin] @ lib^lib:DemoX:CSPROB" self.assertEqual(actual_repr, expected_repr) + + +@ddt +class TestContentLibraryData(TestCase): + """Test the ContentLibraryData class.""" + + @patch("openedx_authz.api.data.ContentLibrary") + def test_get_object_success(self, mock_content_library_model): + """Test get_object returns ContentLibrary when it exists with valid key. + + Expected Result: + - Returns the ContentLibrary object when library exists + - Library key matches exactly (canonical validation passes) + """ + library_id = "lib:DemoX:CSPROB" + library_key = LibraryLocatorV2.from_string(library_id) + library_scope = ContentLibraryData(external_key=library_id) + mock_library_obj = Mock() + mock_library_obj.library_key = library_key + mock_content_library_model.objects.get_by_key.return_value = mock_library_obj + + result = library_scope.get_object() + + self.assertEqual(result, mock_library_obj) + mock_content_library_model.objects.get_by_key.assert_called_once_with(library_key=library_key) + + @patch("openedx_authz.api.data.ContentLibrary") + def test_get_object_does_not_exist(self, mock_content_library_model): + """Test get_object returns None when library does not exist. + + Expected Result: + - Returns None when ContentLibrary.DoesNotExist is raised + """ + library_id = "lib:DemoX:NonExistent" + library_scope = ContentLibraryData(external_key=library_id) + mock_content_library_model.DoesNotExist = Exception + mock_content_library_model.objects.get_by_key.side_effect = mock_content_library_model.DoesNotExist + + result = library_scope.get_object() + + self.assertIsNone(result) + + @patch("openedx_authz.api.data.ContentLibrary") + def test_get_object_invalid_key_format(self, mock_content_library_model): + """Test get_object returns None when library_id has invalid format. + + Expected Result: + - Returns None when InvalidKeyError is raised during key parsing + """ + mock_content_library_model.DoesNotExist = Exception + library_scope = ContentLibraryData(external_key="invalid-library-format") + + result = library_scope.get_object() + + self.assertIsNone(result) + mock_content_library_model.objects.get_by_key.assert_not_called() + + @patch("openedx_authz.api.data.ContentLibrary") + def test_get_object_non_canonical_key(self, mock_content_library_model): + """Test get_object returns None when library key is not canonical. + + This test verifies the canonical key validation: get_by_key is case-insensitive, + but we require exact match to ensure authorization uses canonical library IDs. + + Expected Result: + - Returns None when retrieved library's key doesn't match exactly + - Simulates case where user provides 'lib:demox:csprob' but canonical is 'lib:DemoX:CSPROB' + """ + library_id = "lib:DemoX:CSPROB" + library_key = LibraryLocatorV2.from_string(library_id) + # Convert to lowercase to simulate case-insensitive comparison + library_scope = ContentLibraryData(external_key=library_id.lower()) + mock_content_library_model.objects.get_by_key.return_value = Mock(library_key=library_key) + mock_content_library_model.DoesNotExist = Exception + + result = library_scope.get_object() + + self.assertIsNone(result) + + @patch("openedx_authz.api.data.ContentLibrary") + def test_exists_returns_true_when_library_exists(self, mock_content_library_model): + """Test exists() returns True when get_object() returns a library. + + Expected Result: + - exists() returns True when library object is found + """ + library_id = "lib:DemoX:CSPROB" + library_scope = ContentLibraryData(external_key=library_id) + library_key = LibraryLocatorV2.from_string(library_id) + mock_content_library_model.objects.get_by_key.return_value = Mock(library_key=library_key) + + result = library_scope.exists() + + self.assertTrue(result) + + @patch("openedx_authz.api.data.ContentLibrary") + def test_exists_returns_false_when_library_does_not_exist(self, mock_content_library_model): + """Test exists() returns False when get_object() returns None. + + Expected Result: + - exists() returns False when library is not found + """ + library_id = "lib:DemoX:NonExistent" + library_scope = ContentLibraryData(external_key=library_id) + mock_content_library_model.DoesNotExist = Exception + mock_content_library_model.objects.get_by_key.side_effect = mock_content_library_model.DoesNotExist + + result = library_scope.exists() + + self.assertFalse(result) From 98e2594eef76d49e097463b4920a6d0783b45b00 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 28 Oct 2025 11:35:40 -0500 Subject: [PATCH 2/5] chore: remove unused import from test data file --- openedx_authz/tests/api/test_data.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 55e42260..ca2055e0 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -4,7 +4,6 @@ from ddt import data, ddt, unpack from django.test import TestCase -from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz.api.data import ( From 8dccdf6101b40f9f71d61220fd895de303b007ac Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 28 Oct 2025 11:37:45 -0500 Subject: [PATCH 3/5] chore: bump version to 0.10.0 --- CHANGELOG.rst | 14 ++++++++++++++ openedx_authz/__init__.py | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 49c182fa..26b7d4f6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,20 @@ Unreleased * +0.10.0 - 2025-10-28 +******************* + +Added +===== + +* New ``get_object()`` method in ScopeData to retrieve underlying domain objects +* Implementation of ``get_object()`` for ContentLibraryData with canonical key validation + +Changed +======= + +* Refactor ``ContentLibraryData.exists()`` to use ``get_object()`` internally + 0.9.1 - 2025-10-28 ****************** diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index 07c44b62..bd17868e 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "0.9.1" +__version__ = "0.10.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) From 08179611e6c5869b2940faaddc6801bb2cf5795f Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 28 Oct 2025 12:56:21 -0500 Subject: [PATCH 4/5] feat: add library_key property --- openedx_authz/api/data.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index e1d100f3..7b954d12 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -382,6 +382,15 @@ def library_id(self) -> str: """ return self.external_key + @property + def library_key(self) -> LibraryLocatorV2: + """The LibraryLocatorV2 object for the content library. + + Returns: + LibraryLocatorV2: The library locator object. + """ + return LibraryLocatorV2.from_string(self.library_id) + @classmethod def validate_external_key(cls, external_key: str) -> bool: """Validate the external_key format for ContentLibraryData. @@ -413,11 +422,10 @@ def get_object(self) -> ContentLibrary | None: >>> library_obj = library_scope.get_object() # ContentLibrary object """ try: - library_key = LibraryLocatorV2.from_string(self.library_id) - library_obj = ContentLibrary.objects.get_by_key(library_key=library_key) + library_obj = ContentLibrary.objects.get_by_key(self.library_key) # Validate canonical key: get_by_key is case-insensitive, but we require exact match # This ensures authorization uses canonical library IDs consistently - if library_obj.library_key != library_key: + if library_obj.library_key != self.library_key: raise ContentLibrary.DoesNotExist except (InvalidKeyError, ContentLibrary.DoesNotExist): return None From 55f63b41d1032a71d1509db887cfe1eed38136a6 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 28 Oct 2025 13:03:37 -0500 Subject: [PATCH 5/5] test: update test data to use library_key from ContentLibraryData --- openedx_authz/tests/api/test_data.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index ca2055e0..2925e6b3 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -525,16 +525,15 @@ def test_get_object_success(self, mock_content_library_model): - Library key matches exactly (canonical validation passes) """ library_id = "lib:DemoX:CSPROB" - library_key = LibraryLocatorV2.from_string(library_id) library_scope = ContentLibraryData(external_key=library_id) mock_library_obj = Mock() - mock_library_obj.library_key = library_key + mock_library_obj.library_key = library_scope.library_key mock_content_library_model.objects.get_by_key.return_value = mock_library_obj result = library_scope.get_object() self.assertEqual(result, mock_library_obj) - mock_content_library_model.objects.get_by_key.assert_called_once_with(library_key=library_key) + mock_content_library_model.objects.get_by_key.assert_called_once_with(library_scope.library_key) @patch("openedx_authz.api.data.ContentLibrary") def test_get_object_does_not_exist(self, mock_content_library_model): @@ -598,8 +597,7 @@ def test_exists_returns_true_when_library_exists(self, mock_content_library_mode """ library_id = "lib:DemoX:CSPROB" library_scope = ContentLibraryData(external_key=library_id) - library_key = LibraryLocatorV2.from_string(library_id) - mock_content_library_model.objects.get_by_key.return_value = Mock(library_key=library_key) + mock_content_library_model.objects.get_by_key.return_value = Mock(library_key=library_scope.library_key) result = library_scope.exists()