Skip to content

Commit ba2b528

Browse files
committed
fix: addressing feedback from the PR
1 parent 8575100 commit ba2b528

6 files changed

Lines changed: 69 additions & 8 deletions

File tree

backend/openedx_ai_badges/edxapp_wrapper/contentstore.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,27 @@
22
Contentstore module generalized definitions.
33
"""
44

5+
from functools import lru_cache
56
from importlib import import_module
67

78
from django.conf import settings
89

910

11+
@lru_cache(maxsize=1)
12+
def _get_backend():
13+
"""Return the configured contentstore backend module."""
14+
return import_module(settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND)
15+
16+
1017
def get_static_content():
1118
"""
1219
Wrapper for `xmodule.contentstore.content.StaticContent` in edx-platform.
1320
"""
14-
backend = import_module(settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND)
15-
return backend.StaticContent
21+
return _get_backend().StaticContent
1622

1723

1824
def update_course_run_asset(*args, **kwargs):
1925
"""
2026
Wrapper for `cms.djangoapps.contentstore.views.assets.update_course_run_asset`.
2127
"""
22-
backend = import_module(settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND)
23-
return backend.update_course_run_asset(*args, **kwargs)
28+
return _get_backend().update_course_run_asset(*args, **kwargs)

backend/openedx_ai_badges/processors/badge_image_upload_processor.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,18 @@ def upload_image_to_assets(self, course_id, b64_string, badge_id):
4141
# Strip data URL prefix if present (e.g. "data:image/png;base64,...")
4242
if "," in b64_string:
4343
b64_string = b64_string.split(",", 1)[1]
44-
image_bytes = base64.b64decode(b64_string)
44+
image_bytes = base64.b64decode(b64_string, validate=True)
4545
except Exception: # pylint: disable=broad-except
4646
logger.exception("Failed to decode base64 badge image for badge %s", badge_id)
4747
return None
4848

49+
max_size = getattr(settings, "OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES", 5 * 1024 * 1024)
50+
if len(image_bytes) > max_size:
51+
logger.error(
52+
"Badge image for badge %s exceeds maximum allowed size (%d bytes)", badge_id, max_size
53+
)
54+
return None
55+
4956
filename = f"badge_{badge_id}.png"
5057
file_obj = InMemoryUploadedFile(
5158
file=io.BytesIO(image_bytes),

backend/openedx_ai_badges/settings/common.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ def plugin_settings(settings):
3030
if badges_workflow_dir not in settings.WORKFLOW_TEMPLATE_DIRS:
3131
settings.WORKFLOW_TEMPLATE_DIRS.append(badges_workflow_dir)
3232

33-
# Wrappers for openedx-platform functions
33+
# -------------------------
34+
# Contentstore wrapper
35+
# -------------------------
3436
settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND = (
3537
"openedx_ai_badges.edxapp_wrapper.backends.contentstore_r_v1"
3638
)
39+
settings.OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES = 1 * 1024 * 1024 # 1 MB
3740

3841
# -------------------------
3942
# MIT DCC Badge API

backend/openedx_ai_badges/settings/production.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,14 @@ def plugin_settings(settings):
3131
settings.MIT_DCC_BADGE_API_HEALTH_URL = settings.ENV_TOKENS.get(
3232
"MIT_DCC_BADGE_API_HEALTH_URL", settings.MIT_DCC_BADGE_API_HEALTH_URL
3333
)
34+
35+
# -------------------------
36+
# Contentstore wrapper
37+
# -------------------------
38+
if hasattr(settings, "ENV_TOKENS"):
39+
settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND = settings.ENV_TOKENS.get(
40+
"OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND", settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND
41+
)
42+
settings.OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES = settings.ENV_TOKENS.get(
43+
"OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES", settings.OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES
44+
)

backend/tests/test_badge_image_upload_processor.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ def test_skips_conversion_when_already_course_key(self, processor, settings):
7878

7979
with patch("openedx_ai_badges.processors.badge_image_upload_processor.update_course_run_asset", mock_upload), \
8080
patch("openedx_ai_badges.processors.badge_image_upload_processor.get_static_content", mock_get_static), \
81-
patch("openedx_ai_badges.processors.badge_image_upload_processor.CourseKey") as mock_ck:
81+
patch.object(CourseKey, "from_string") as mock_from_string:
8282
processor.upload_image_to_assets(real_key, MINIMAL_PNG_B64, BADGE_ID)
83-
mock_ck.from_string.assert_not_called()
83+
mock_from_string.assert_not_called()
8484

8585
assert mock_upload.call_args[0][0] is real_key
8686

@@ -106,6 +106,18 @@ def test_returns_none_on_upload_failure(self, processor, settings):
106106

107107
assert url is None
108108

109+
def test_returns_none_when_image_exceeds_max_size(self, processor, settings):
110+
settings.LMS_ROOT_URL = LMS_ROOT
111+
settings.OPENEDX_AI_BADGES_MAX_IMAGE_SIZE_BYTES = 1 # 1 byte limit
112+
mock_upload, mock_get_static = _make_upload_mock()
113+
114+
with patch("openedx_ai_badges.processors.badge_image_upload_processor.update_course_run_asset", mock_upload), \
115+
patch("openedx_ai_badges.processors.badge_image_upload_processor.get_static_content", mock_get_static):
116+
url = processor.upload_image_to_assets(COURSE_ID_STR, MINIMAL_PNG_B64, BADGE_ID)
117+
118+
assert url is None
119+
mock_upload.assert_not_called()
120+
109121
def test_uploaded_file_has_correct_name_and_content_type(self, processor, settings):
110122
settings.LMS_ROOT_URL = LMS_ROOT
111123
mock_upload, mock_get_static = _make_upload_mock()

backend/tests/test_contentstore_wrapper.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,17 @@
88

99
from unittest.mock import MagicMock, patch
1010

11+
import pytest
12+
13+
14+
@pytest.fixture(autouse=True)
15+
def clear_backend_cache():
16+
"""Clear the lru_cache on _get_backend between tests."""
17+
from openedx_ai_badges.edxapp_wrapper.contentstore import _get_backend
18+
_get_backend.cache_clear()
19+
yield
20+
_get_backend.cache_clear()
21+
1122

1223
class TestGetStaticContent:
1324

@@ -34,6 +45,18 @@ def test_uses_configured_backend_setting(self, settings):
3445

3546
mock_import.assert_called_once_with("my.custom.backend")
3647

48+
def test_backend_is_cached_across_calls(self, settings):
49+
fake_backend = MagicMock()
50+
fake_backend.StaticContent = MagicMock()
51+
settings.OPENEDX_AI_BADGES_CONTENTSTORE_BACKEND = "fake.backend"
52+
53+
with patch("openedx_ai_badges.edxapp_wrapper.contentstore.import_module", return_value=fake_backend) as mock_import:
54+
from openedx_ai_badges.edxapp_wrapper.contentstore import get_static_content
55+
get_static_content()
56+
get_static_content()
57+
58+
mock_import.assert_called_once()
59+
3760

3861
class TestUpdateCourseRunAsset:
3962

0 commit comments

Comments
 (0)