Skip to content

Commit 4de24a8

Browse files
committed
feat: implement submission file workflow with comprehensive testing
- Create Submission file model - Add classes to handle file errors - Develop SubmissionFileProcessor for create files in DB - Add TestSubmissionFileProcessor with extensive test coverage - Test file processing with different input types (bytes, file objects) - Implement error handling tests for IO errors and invalid files - Verify complete file processing workflow - Validation tests for file addition in create_external_grader_detail - Document architectural decisions with ADR - Add test queue folder in .gitignore
1 parent a904823 commit 4de24a8

8 files changed

Lines changed: 672 additions & 5 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,4 @@ test.txt.tmp
6363

6464
# VSCode
6565
.vscode
66+
test_queue/*
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
2. File Handling Implementation for Submission System
2+
#####################################################
3+
4+
Status
5+
******
6+
7+
**Provisional** *2025-02-14*
8+
9+
Implemented by https://github.com/openedx/edx-submissions/pull/286
10+
11+
Context
12+
*******
13+
14+
As part of the XQueue migration effort detailed in ADR 0001, we need to implement a file handling system within edx-submissions. Currently, XQueue manages file submissions through a tightly coupled approach.
15+
16+
### Current Limitations
17+
18+
1. **Inadequate File Management**: XQueue's approach relies on JSON strings in character fields, with size constraints and manual URL manipulation for file handling.
19+
20+
2. **Process Inefficiencies**: The current system uses synchronous HTTP for file retrieval, lacks proper validation, and tightly couples submission processing with file handling.
21+
22+
3. **Integration Challenges**: External graders depend on specific URL formats with HTTP-based retrieval, embedding file information directly in submission payloads.
23+
24+
Decision
25+
********
26+
27+
We will implement a dedicated file management system for the assessment submission process, focusing on workflow and educational needs:
28+
29+
1. **Centralized Storage**: Create a unified repository for student-submitted files, ensuring they are properly associated with their assessments and accessible throughout the grading process.
30+
31+
2. **Streamlined Workflow**: Design a clear process where files are automatically processed during submission creation, securely stored, and efficiently delivered to grading systems.
32+
33+
3. **Consistent Experience**: Maintain compatibility with existing systems to ensure a smooth transition, allowing instructors and external graders to access files without changes to their established workflows.
34+
35+
Consequences
36+
************
37+
38+
Positive:
39+
---------
40+
41+
1. **Architecture**: Clean separation of concerns, improved maintainability, better error handling, optimized database access
42+
43+
2. **Integration**: Seamless xqueue-watcher compatibility, support for existing workflows, minimal client code changes
44+
45+
3. **Operations**: Robust file validation, improved tracking, better error visibility, simplified lifecycle management
46+
47+
Negative:
48+
---------
49+
50+
1. **Technical**: Additional database structures
51+
52+
2. **Migration**: Temporary system complexity, additional testing needs
53+
54+
3. **Performance**: File processing overhead
55+
56+
References
57+
**********
58+
59+
Current System Documentation:
60+
* XQueue Repository: https://github.com/openedx/xqueue
61+
* XQueue Watcher Repository: https://github.com/openedx/xqueue-watcher
62+
63+
Related Repositories:
64+
* edx-submissions: https://github.com/openedx/edx-submissions
65+
* edx-platform: https://github.com/openedx/edx-platform
66+
* XQueue Repository: https://github.com/openedx/xqueue
67+
68+
Related Documentation:
69+
* ADR 0001: Creation of ExternalGraderDetail Model for XQueue Migration
70+
71+
Future Event Integration:
72+
* Open edX Events Framework: https://github.com/openedx/openedx-events
73+
* Event Bus Documentation: https://openedx.atlassian.net/wiki/spaces/AC/pages/124125264/Event+Bus
74+
75+
Related Architecture Documents:
76+
* Open edX Architecture Guidelines: https://openedx.atlassian.net/wiki/spaces/AC/pages/124125264/Architecture+Guidelines
77+

submissions/api.py

Lines changed: 104 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010

1111
from django.conf import settings
1212
from django.core.cache import cache
13+
from django.core.files.base import ContentFile
1314
from django.db import DatabaseError, IntegrityError, transaction
1415

1516
# SubmissionError imported so that code importing this api has access
1617
from submissions.errors import ( # pylint: disable=unused-import
1718
ExternalGraderQueueEmptyError,
19+
FileProcessingError,
1820
SubmissionError,
1921
SubmissionInternalError,
2022
SubmissionNotFoundError,
@@ -28,6 +30,7 @@
2830
ScoreSummary,
2931
StudentItem,
3032
Submission,
33+
SubmissionFile,
3134
score_reset,
3235
score_set
3336
)
@@ -49,7 +52,6 @@
4952
TOP_SUBMISSIONS_CACHE_TIMEOUT = 300
5053

5154

52-
# pylint: disable=unused-argument
5355
def create_external_grader_detail(student_item_dict,
5456
answer,
5557
queue_name: str,
@@ -93,9 +95,13 @@ def create_external_grader_detail(student_item_dict,
9395
submission_uuid=submission_uuid,
9496
queue_name=queue_name,
9597
grader_file_name=grader_file_name,
96-
points_possible=points_possible,
98+
points_possible=points_possible)
99+
100+
files_dict = external_grader_additional_data.get('files')
101+
if files_dict:
102+
file_processor = SubmissionFileProcessor(instance)
103+
file_processor.process_files(files_dict)
97104

98-
)
99105
return instance
100106

101107
except DatabaseError as error:
@@ -106,6 +112,101 @@ def create_external_grader_detail(student_item_dict,
106112
raise SubmissionInternalError(error_message) from error
107113

108114

115+
class SubmissionFileProcessor:
116+
"""
117+
Process file operations for submissions
118+
"""
119+
120+
def __init__(self, external_grader):
121+
self.external_grader = external_grader
122+
123+
def process_files(self, files_dict):
124+
"""
125+
Process uploaded files from an Open edX environment and store them as SubmissionFile objects.
126+
127+
This method handles various file object types that might be received from Open edX, including:
128+
- Bytes objects
129+
- Any object with a 'read' method (including Native Open edX FileObjForWebobFiles objects)
130+
131+
132+
The method performs the following operations:
133+
1. Validates each file object typeƒ
134+
2. Reads content from file-like objects
135+
3. Converts byte content to ContentFile objects
136+
4. Creates SubmissionFile records in the database
137+
5. Returns URLs in xqueue-compatible format
138+
139+
Args:
140+
files_dict (dict): Dictionary mapping filenames to file objects.
141+
Format: {filename: file_object, ...}
142+
143+
Returns:
144+
dict: Dictionary mapping original filenames to xqueue URLs.
145+
Format: {filename: "/queue_name/uuid", ...}
146+
147+
Raises:
148+
FileProcessingError: If there's an error reading from a file object,
149+
including I/O errors or Unicode decoding errors.
150+
151+
Example:
152+
>>> external_grader = ExternalGraderDetail.create_from_uuid(
153+
submission_uuid=submission_uuid,
154+
queue_name=queue_name,
155+
grader_file_name=grader_file_name,
156+
points_possible=points_possible)
157+
>>> file_processor = SubmissionFileProcessor(external_grader)
158+
>>> files = {'assignment.py': file_obj}
159+
>>> urls = file_processor.process_files(files)
160+
>>> print(urls)
161+
{'assignment.py': '/my_queue/550e8400-e29b-41d4-a716-446655440000'}
162+
"""
163+
files_urls = {}
164+
for filename, file_obj in files_dict.items():
165+
# Validate file object type
166+
if not (isinstance(file_obj, bytes) or hasattr(file_obj, 'read')):
167+
actual_type = type(file_obj).__name__
168+
logger.error(f"Invalid file object type for {filename}: got {actual_type}")
169+
raise TypeError(
170+
f"Invalid file object type for {filename}: expected bytes or an object with 'read' method, "
171+
f"got {actual_type}")
172+
173+
# Handle file-like objects by reading their content
174+
# This returns a bytes object that will be converted to ContentFile later
175+
if hasattr(file_obj, 'read'):
176+
try:
177+
file_obj = file_obj.read() # read() returns a bytes object
178+
except (IOError, OSError) as e:
179+
logger.error(f"Error reading file {filename}: {e}")
180+
raise FileProcessingError(f"Error reading file {filename}: {e}") from e
181+
except UnicodeDecodeError as e:
182+
logger.error(f"Error decoding file {filename}: {e}")
183+
raise FileProcessingError(f"Error decoding file {filename}: {e}") from e
184+
185+
# Convert bytes to ContentFile
186+
# The read() method from file-like objects returns bytes, which we handle here
187+
if isinstance(file_obj, bytes):
188+
file_obj = ContentFile(file_obj, name=filename)
189+
190+
# Create a SubmissionFile record for storage and retrieval
191+
submission_file = SubmissionFile.objects.create(
192+
external_grader=self.external_grader,
193+
file=file_obj,
194+
original_filename=filename
195+
)
196+
files_urls[filename] = submission_file.xqueue_url
197+
198+
return files_urls
199+
200+
def get_files_for_grader(self):
201+
"""
202+
Returns files in format expected by xqueue-watcher
203+
"""
204+
return {
205+
file.original_filename: file.file.url
206+
for file in self.external_grader.files.all()
207+
}
208+
209+
109210
def create_submission(
110211
student_item_dict,
111212
answer,

submissions/errors.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,17 @@ class ExternalGraderQueueEmptyError(SubmissionError):
4040
"""
4141

4242

43+
class FileProcessingError(SubmissionError):
44+
"""
45+
Exception raised when there's an error reading or processing a file.
46+
47+
This exception is raised when file operations fail, such as:
48+
- I/O errors when reading file content
49+
- OS errors during file operations
50+
- Unicode decoding errors when processing file content
51+
"""
52+
53+
4354
class SubmissionRequestError(SubmissionError):
4455
"""
4556
This error is raised when there was a request-specific error
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Generated by Django 4.2.19 on 2025-04-21 13:20
2+
3+
from django.db import migrations, models
4+
import django.db.models.deletion
5+
import django.utils.timezone
6+
import submissions.models
7+
import uuid
8+
9+
10+
class Migration(migrations.Migration):
11+
12+
dependencies = [
13+
('submissions', '0004_externalgraderdetail'),
14+
]
15+
16+
operations = [
17+
migrations.CreateModel(
18+
name='SubmissionFile',
19+
fields=[
20+
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
21+
('uuid', models.UUIDField(default=uuid.uuid4, editable=False)),
22+
('file', models.FileField(max_length=512, upload_to=submissions.models.submission_file_path)),
23+
('original_filename', models.CharField(max_length=255)),
24+
('created_at', models.DateTimeField(default=django.utils.timezone.now)),
25+
('external_grader', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL,
26+
related_name='files', to='submissions.externalgraderdetail')),
27+
],
28+
options={
29+
'indexes': [models.Index(fields=['external_grader', 'uuid'], name='submissions_externa_ff8089_idx')],
30+
},
31+
),
32+
]

submissions/models.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"""
1111

1212
import logging
13+
import os
1314
from datetime import timedelta
1415
from uuid import uuid4
1516

@@ -693,3 +694,59 @@ def update_status(self, new_status):
693694
def create_from_uuid(cls, submission_uuid, **kwargs):
694695
submission = Submission.objects.get(uuid=submission_uuid)
695696
return cls.objects.create(submission=submission, **kwargs)
697+
698+
699+
def submission_file_path(instance, _):
700+
"""
701+
Generate file path for submission files.
702+
Format: queue_name/uuid
703+
The filename is replaced with the UUID to ensure uniqueness without preserving extension.
704+
"""
705+
return os.path.join(
706+
instance.external_grader.queue_name,
707+
f"{instance.uuid}"
708+
)
709+
710+
711+
class SubmissionFile(models.Model):
712+
"""
713+
Model to handle files associated with submissions
714+
"""
715+
uuid = models.UUIDField(default=uuid4, editable=False) # legacy S3 key
716+
external_grader = models.ForeignKey(
717+
'submissions.ExternalGraderDetail',
718+
on_delete=models.SET_NULL,
719+
related_name='files',
720+
null=True,
721+
)
722+
file = models.FileField(
723+
upload_to=submission_file_path,
724+
max_length=512
725+
)
726+
original_filename = models.CharField(max_length=255) # This is necessary to send file name to xqueue-watcher
727+
created_at = models.DateTimeField(default=now)
728+
729+
class Meta:
730+
indexes = [
731+
models.Index(fields=['external_grader', 'uuid']),
732+
]
733+
734+
@property
735+
def xqueue_url(self):
736+
"""
737+
Returns a URL in the XQueue-compatible format: /queue_name/uuid
738+
739+
This format is used for file references in both the legacy XQueue system
740+
and the new integrated standard. It maintains backward compatibility
741+
while supporting the migration from the external XQueue API to the
742+
integrated Open edX solution.
743+
744+
The URL follows the pattern: /{queue_name}/{submission_uuid}
745+
where:
746+
- queue_name: identifies the external grader queue
747+
- uuid: uniquely identifies this submission (legacy S3 key)
748+
749+
Returns:
750+
str: Formatted URL path following XQueue conventions
751+
"""
752+
return f"/{self.external_grader.queue_name}/{self.uuid}"

0 commit comments

Comments
 (0)